RFR: 8284330: jcmd may not be able to find processes in the container [v2]

[Ioi Lam]

On Wed, 6 Apr 2022 12:44:35 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:

>> jcmd uses src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java to scan temporary directories to find out processes in the container. It checks inode to ensure the temp directory is not conflicted. However inode maybe same value between the container and others. Thus we should check device id for that case.
>> 
>> For example I saw following case on [distroless cc-debian11](https://github.com/GoogleContainerTools/distroless/blob/main/cc/README.md) container. I started rescue:jdk19 container with sharing PID namespace. `/proc/1/root/tmp` is different from `/tmp` on rescue:jdk19, but they are same inode value. However we can see the differense in device id.
>> 
>> 
>> $ podman run -it --rm --entrypoint=sh --pid=container:fa39662f7352 rescue:jdk19
>> / #
>> / # stat /tmp
>>   File: /tmp
>>   Size: 29              Blocks: 0          IO Block: 4096   directory
>> Device: efh/239d        Inode: 135674931   Links: 1
>> Access: (1777/drwxrwxrwt)  Uid: (    0/    root)   Gid: (    0/    root)
>> Access: 2022-04-05 08:51:37.000000000
>> Modify: 2022-04-05 08:51:37.000000000
>> Change: 2022-04-05 08:51:37.000000000
>> 
>> / # stat /proc/1/root/tmp
>>   File: /proc/1/root/tmp
>>   Size: 29              Blocks: 0          IO Block: 4096   directory
>> Device: e1h/225d        Inode: 135674931   Links: 1
>> Access: (1777/drwxrwxrwt)  Uid: (    0/    root)   Gid: (    0/    root)
>> Access: 2022-04-05 08:51:37.000000000
>> Modify: 2022-04-05 08:50:42.000000000
>> Change: 2022-04-05 08:50:42.000000000
>
> Yasumasa Suenaga has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix comments

The code changes look good, but I think the comment should be cleaned up.

src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java line 117:

> 115:      * skip these duplicated directories.
> 116:      * Host and container devices could have the same inode value,
> 117:      * so we also need to check the device id.

I would suggest rewording the comments from Line 111 to 117 to the following to be more concise:


     * When cgroups is enabled, the directory /proc/{pid}/root/tmp may
     * exist even if the given pid is not running inside a container. In
     * this case, this directory is usually the same as /tmp and should
     * be skipped, or else we would get duplicated hsperfdata files.
     * This case can be detected if the inode and device id of
     * /proc/{pid}/root/tmp are the same as /tmp.

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

Changes requested by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8103