Re: FW: 8225543: Jcmd fails to attach to the Java process on Linux using the main class name if whitespace options were used to launch the process

[serguei.spit...@oracle.com]

Hi Daniil, I'm confused with the change: for (int i = 1; i < parts.length && mainClass == null; i++) { if (i < parts.length - 1) { if (parts[i].equals("-m") || parts[i].equals("--module") || parts[i].equals("-jar")) { <= ?????? return parts[i + 1]; } } - // If this is a classpath or a module path option then skip the next part - // (the classpath or the module path itself) + + if ((parts[i].startsWith("--module="))) { <= ?????? + return parts[i].substring("--module=".length()); + } + + // If this is a classpath or a module whitespace option then skip the next part + // (the classpath or the option value itself) if (parts[i].equals("-cp") || parts[i].equals("-classpath") || parts[i].equals("--class-path") || - parts[i].equals("-p") || parts[i].equals("--module-path")) { + isModuleWhiteSpaceOption(parts[i])) { . . . + private static boolean isModuleWhiteSpaceOption(String option) { + return option.equals("-p") || + option.equals("--module-path") || How these changes are going to work for options "--module=" and "--module-path" if there is this check at the beginning of each iteration:   parts[i].equals("--module") ? Also, this line has unneeded extra "()":   + if ((parts[i].startsWith("--module="))) { Thanks, Serguei On 6/12/19 10:50, Daniil Titov wrote: Hi David and Serguei, Could you please have a look at this change? This fix is related with the changes [3] that you reviewed back in February. Mach5 sun/tools/jcmd, serviceability/dcmd/framework, jdk_svc, hotspot_serviceability, tier1, tier2 and tier3 tests passed. Thanks a lot! --Daniil On 6/10/19, 4:56 PM, "serviceability-dev on behalf of Daniil Titov" <serviceability-dev-boun...@openjdk.java.net on behalf of daniil.x.ti...@oracle.com> wrote: Please review the change that fixes jcmd process name matching on Linux platform. After changes [3] , on Linux platform the proc filesystem is used to find a Java process, however, sun.tools.ProcessHelper class, introduced in that change, didn't take into account the presence of module whitespace options ( e.g. --patch-module) or the fact that some values of Java options could contain whitespaces. The latter problem is fixed by keeping '\0' as a separator for command line arguments read from /proc/{pid}/cmdline rather than replacing it with a whitespace. The former problem is fixed by making sun.tools.ProcessHelper aware of these module whitespace options (--add-opens, --add-exports,--add-reads, --add-modules, --patch-module,--limit-modules, or --upgrade-module-path) as well as of the case when a source-file mode is used or when --module options is used in "--<name>=<value>" format. Testing: sun/tools/jcmd and serviceability/dcmd/framework tests succeeded in Mach5. jdk_svc, tier1, tier2, and tier3 tests are in progress. [1] Webrev: http://cr.openjdk.java.net/~dtitov/8225543/webrev.01/ [2] Bug: https://bugs.openjdk.java.net/browse/JDK-8225543 [3] https://bugs.openjdk.java.net/browse/JDK-8205654 Thanks! --Daniil