RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[David Holmes]

Hi Bob,

Some initial discussion and commentary. I'm a bit concerned by how much 
accommodating docker penalises code that is not using containers.

On 12/01/2018 4:41 AM, Bob Vandette wrote:
> Bug:
> 
> https://bugs.openjdk.java.net/browse/JDK-8193710
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~bobv/8193710/webrev.00/
> 
> Summary:
> 
> This changeset enables the ability to use jcmd and jps running on a Host to
> list the java processes that are running in docker (cgroup based) containers.

Can you clarify this for me please. I assume we are still only listing 
processes for the current user - correct? So the issue is if I am 
running mutiple JVMs and also running a Docker process that itself 
contains JVMs then jps outside of Docker won't show the JVMs that are 
inside Docker. And presumably the opposite is also true?

If we change jps to find all JVMs on the host for the current user, 
whether in a container or not (and whether jps is run from in a 
container or not?) are the process id's that get returned directly 
useable from where jps was run?

> I’ve tested this change by examining processes running as root on both host and in
> docker containers as well as under my userid using “jps and jcmd -l”.
> I’ve also tested the getFile functions with a small example program that I wrote.
>  From what I can tell,  these two getFile functions are not used in the JDK sources
> and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to have never worked!
> I’d really like to remove these two methods.

If they are not used by anything then by all means drop them.

>   140                 candidate = new File(f.getName(), name);  <<——— This line drops the /tmp directory off of the path.
> 
> Here are some implementation details that I’ve added added to one of the Unix
> specific source files (src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)

I have an issue with this. This is not "unix" (ie anything other than 
windows) support code, it is Linux support code. I'm also unclear why 
this needs to be lifted out of the PerfMemory subsystem.

And if you're going to mess with the jdk.internal module then I think 
core-libs folk will want to know before hand.

Other comments ...

src/hotspot/os/linux/perfMemory_linux.cpp

I'm concerned about the MAXPATHLEN stack buffers and the extra overhead 
being added to the normal non-container path. We've had issues in the 
past where a newly added stack buffer caused StackOverflowError. We 
don't need a 4K buffer because os::get_temp_directory() just returns 
"/tmp" and we're dealing with all Linux code here. I don't like making 
assumptions like this but ...

#define TMP_BUFFER_LEN (4 + 21)
static char* get_user_tmp_dir(const char* user, int vmid, int nspid) {
   char buffer[TMP_BUFFER_LEN];
   char* tmpdir = os::get_temp_directory();
   assert(strlen(tmpdir) == 4, "No longer using /tmp - update buffer size");
   if (nspid != -1) {
     jio_snprintf(buffer, TMP_BUFFER_LEN, "/proc/%d/root%s", vmid, tmpdir);
     tmpdir = buffer;
   }
   ...


657   char fname[64];

Maximum length of "/proc/%d/status" is 23.

Thanks,
David