Hi Serguei,
>> 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")
Let me describe in details these cases you mentioned.
Case 1:
---------
part[i] is "--module"
part[i+1] is "myModule/MyClass"
Case 2:
---------
part[i] is "--module=myModule/MyClass "
Case 3:
---------
part[i] is "--module-path"
part[i+1] is "mods"
For case 1 we have the condition on line 94 that is evaluated to TRUE and we
return the next part on line 95 (part[i+1]) as a module and class name.
For case 2 the condition on line 94 is evaluated to FALSE so we continue to
line 99. On line 99 the condition is evaluated to TRUE so on line 100 we
return the value part of the option (everything after "--module=") as a module
and class name. Please note the condition on line 94 uses
equals() while on the line 99 it uses startsWith().
For case 3 the condition on lines 94 and 99 are evaluated to FALSE so we
continue to lines 105/106 and isModuleWhiteSpaceOption(parts[i]) on line 106
returns TRUE. As a result, we skip this and the next parts and continue to the
beginning of the loop at line 92.
92 for (int i = 1; i < parts.length && mainClass == null; i++) {
93 if (i < parts.length - 1) {
94 if (parts[i].equals("-m") ||
parts[i].equals("--module") || parts[i].equals("-jar")) {
95 return parts[i + 1];
96 }
97 }
98
99 if ((parts[i].startsWith("--module="))) {
100 return parts[i].substring("--module=".length());
101 }
102
103 // If this is a classpath or a module whitespace option
then skip the next part
104 // (the classpath or the option value itself)
105 if (parts[i].equals("-cp") || parts[i].equals("-classpath")
|| parts[i].equals("--class-path") ||
106 isModuleWhiteSpaceOption(parts[i])) {
107 i++;
108 continue;
109 }
110
>> Also, this line has unneeded extra "()":
>> + if ((parts[i].startsWith("--module="))) {
Yes, you are right! Thanks! I will remove it before pushing or in the new
version of a webrev if we will come to it.
Thank you!
-Daniil
From: "serguei.spit...@oracle.com" <serguei.spit...@oracle.com>
Date: Wednesday, June 12, 2019 at 12:39 PM
To: Daniil Titov <daniil.x.ti...@oracle.com>, David Holmes
<david.hol...@oracle.com>
Cc: OpenJDK Serviceability <serviceability-dev@openjdk.java.net>
Subject: 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
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"
mailto:serviceability-dev-boun...@openjdk.java.netonbehalfofdaniil.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
