On Mon, 15 Jun 2026 10:02:51 GMT, Kevin Walls <[email protected]> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use shouldMatch for logging message.
>
> There is also a problem if a Java process in a container uses AltTempDir, a 
> jps/jcmd process listing from the host cannot see the processes, even if 
> setting the same AltTempDir (jps in the host usually lists Java processes 
> running in containers).
> 
> "open/src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java"
>  needs to update getLocalVmId() as it contains knowledge of the /tmp path to 
> find hsperdata, which contains host pid and namespace (in-container) pid.
> 
> Suggestion below which works me for (but is not foolproof...).
> 
> 
> One more issue: run in a container, with AltTempDir set to a directory 
> mounted (shared) from the host.
> When the host runs jps with AltTempDir set the same, 
> PlatformSupportImpl.java/getTemporaryDirectories() ignores some directories, 
> because they are the same as its tmp dir (this was intentional, but becomes a 
> problem in this case).
> 
> 
> $ build/linux-x64/images/jdk/bin/jps  -J-XX:AltTempDir=/work
> 2880104 Jps
> 111 -- process information unavailable
> 
> 
> This means a /proc/2878800/root/work/hsperfdata_kwalls/111 path got ignored 
> in preference for /work/hsperfdata_kwalls/111 ..from which it can't deduce 
> both hostpid and nspid.
> 
> Removing that check for the files being the same in getTemporaryDirectories() 
> works:
> 
> 
> --- 
> a/src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java
> +++ 
> b/src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java
> @@ -159,9 +159,8 @@ public boolean accept(File dir, String name) {
>              File containerFile = new File(containerTmpDir);
> 
>              if (containerFile.exists() && containerFile.isDirectory() &&
> -                containerFile.canRead() &&
> -                !tempDirectoryEquals(containerFile.toPath())) {
> -                v.add(containerTmpDir);
> +                containerFile.canRead()) {
> +                   v.add(containerTmpDir);
>              }
>          }
> 
> 
> ...although it shows an error in jps:
> 
> 
> $ build/linux-x64/images/jdk/bin/jps -J-XX:AltTempDir=/work
> 145 -- process information unavailable
> 2883758 ThreadsMem
> 2884405 Jps
> 
> 
> 2883758 and 145 are the same process.
> 
> jcmd will ignore this trivial failure to attach (as processes may come and 
> go...) and looks fine.
> 
> Making jps behave the same as jcmd by ignoring this error is what I came up 
> with:
> 
>  
> 
> src/jdk.jcmd/share/classes/sun/tools/jps/Jps.java
> @@ -87,7 +86,6 @@ public static void main(String[] args) {
>                      // reasonable message to indicate that the requested
>                      // info...

For an attempt at clarity let me summarise my concern/confusion here so that 
@kevinjwalls can address it. One thing about this temp handling is that it 
covers two distinct cases:

- the temp directory of the current VM (where do I put my hsperf data for 
example)
- where to look for the temp directory of a target VM (e.g. for attach)

There is no reason these two locations have to be the same, but we are treating 
the same - that's okay. 

My confusion/concern is that given a temp directory `/foo` if we are using it 
internally to find the current VM's hsperf data then the path is simply 
`/foo/.java_pid{pid}`. But if we are looking for a target VMs temp dir and in a 
container then it may actually be `/proc/{vmid}/root/foo`. In the former case 
we need to allow for the suffix length of `/.java_pid{pid}` which is 20 (nul 
not included). For the attach prefix we need to allow for a length of 22 (nul 
included). To further complicate things only the attach case is limited overall 
to UNIX_PATH_MAX (the other is just PATH_MAX ie 4096). So do we ever have to 
allow for both the prefix and the suffix in the attach case or are they 
disjoint cases?

If we have to allow for both then we need to use `UNIX_PATH_MAX-42` as the safe 
max.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/31407#issuecomment-4837334986

Reply via email to