RFR 8205654: serviceability/dcmd/framework/HelpTest.java timed out

Discussion:

Daniil Titov

2018-11-10 05:18:41 UTC

Please review the change that fixes serviceability/dcmd/framework/* tests from a time out. The fix for JDK-8166642 made serviceability/dcmd/framework/* tests non-concurrent to ensure that they don't interact with each other and there are no multiple tests running simultaneously since all they do share the common main class name com.sun.javatest.regtest.agent.MainWrapper. However, it looks like the tests from other directories still might run in parallel with these tests and they also have com.sun.javatest.regtest.agent.MainWrapper as a main class.

The fix ensures that each serviceability/dcmd/framework/* test uses a Java process with a unique main class name when connecting to this process with jcmd and the main class name.

Bug: https://bugs.openjdk.java.net/browse/JDK-8205654
Webrev: http://cr.openjdk.java.net/~dtitov/8205654/webrev.001/

Best regards,
Daniil

g***@oracle.com

2018-11-10 15:59:32 UTC

Permalink

Looks good to me.

You'll need the build directive in HelpTest and InvalidCommandTest.

Post by Daniil Titov
Please review the change that fixes serviceability/dcmd/framework/* tests from a time out. The fix for JDK-8166642 made serviceability/dcmd/framework/* tests non-concurrent to ensure that they don't interact with each other and there are no multiple tests running simultaneously since all they do share the common main class name com.sun.javatest.regtest.agent.MainWrapper. However, it looks like the tests from other directories still might run in parallel with these tests and they also have com.sun.javatest.regtest.agent.MainWrapper as a main class.
The fix ensures that each serviceability/dcmd/framework/* test uses a Java process with a unique main class name when connecting to this process with jcmd and the main class name.
Bug: https://bugs.openjdk.java.net/browse/JDK-8205654
Webrev: http://cr.openjdk.java.net/~dtitov/8205654/webrev.001/
Best regards,
Daniil

s***@oracle.com

2018-11-10 23:40:32 UTC

Permalink

Hi Daniil,

It looks Okay to me.

Thanks,
Serguei

David Holmes

2018-11-11 21:35:30 UTC

Permalink

Hi Daniil,

I took a quick look at this one ... two minor comments

The static class names could just be "Process" as they will acquire the
enclosing class name as part of their own name anyway. As it is this
gets repeated eg:

HelpTest$HelpTestProcess
InvalidCommandTest$InvalidCommandTestProcess

TestJavaProcess.java:

39 public static void main(String argv[]) {

Nit: Should be "String[] argv" in Java style

Thanks,
David

Daniil Titov

2018-11-29 21:30:18 UTC

Permalink

Thank you, David!

The proposed fix didn't help. It still hangs at some occasions. Additional tracing showed that when jcmd is invoked with the main class name it iterates over all running Java processes and temporary attaches to them to retrieve the main class name. It hangs while trying to attach to one of the running Java processes. There are numerous Java processes running at the host machine some associated with the test framework itself and another with the tests running in parallel. It is not clear what exact is this particular process since the jcmd hangs before retrieving the process' main class name, but after all tests terminated the process with this id is no longer running. I have to revoke this review since more investigation is required.

Best regards,
Daniil

On 11/11/18, 1:35 PM, "David Holmes" <***@oracle.com> wrote:

Hi Daniil,

I took a quick look at this one ... two minor comments

The static class names could just be "Process" as they will acquire the
enclosing class name as part of their own name anyway. As it is this
gets repeated eg:

HelpTest$HelpTestProcess
InvalidCommandTest$InvalidCommandTestProcess

TestJavaProcess.java:

39 public static void main(String argv[]) {

Nit: Should be "String[] argv" in Java style

Thanks,
David

David Holmes

2018-11-30 00:52:17 UTC

Permalink

Hi Daniil,

That sounds like an unsolvable problem for the test. You can't control
other Java processes on the machine, and searching by name requires
asking each of them in turn.

How do we get the list of Java processes in the first place? Perhaps we
need to do some /proc/<pid>/cmdline peeking?

Cheers,
David

Post by Daniil Titov
Best regards,
Daniil
Hi Daniil,
I took a quick look at this one ... two minor comments
The static class names could just be "Process" as they will acquire the
enclosing class name as part of their own name anyway. As it is this
HelpTest$HelpTestProcess
InvalidCommandTest$InvalidCommandTestProcess
39 public static void main(String argv[]) {
Nit: Should be "String[] argv" in Java style
Thanks,
David

Chris Plummer

2018-11-30 01:16:28 UTC

Permalink

Can you remind me why the test is trying to attach to all running java
processes.

Chris

Post by Daniil Titov
Thank you, David!
The proposed fix didn't help. It still hangs at some occasions. Additional tracing showed that when jcmd is invoked with the main class name it iterates over all running Java processes and temporary attaches to them to retrieve the main class name. It hangs while trying to attach to one of the running Java processes. There are numerous Java processes running at the host machine some associated with the test framework itself and another with the tests running in parallel. It is not clear what exact is this particular process since the jcmd hangs before retrieving the process' main class name, but after all tests terminated the process with this id is no longer running. I have to revoke this review since more investigation is required.
Best regards,
Daniil
Hi Daniil,
I took a quick look at this one ... two minor comments
The static class names could just be "Process" as they will acquire the
enclosing class name as part of their own name anyway. As it is this
HelpTest$HelpTestProcess
InvalidCommandTest$InvalidCommandTestProcess
39 public static void main(String argv[]) {
Nit: Should be "String[] argv" in Java style
Thanks,
David

Daniil Titov

2018-11-30 02:12:30 UTC

Permalink

Hi Chris,

It attaches to the instrumentation buffer when it is getting a MonitoredVm object inside check() method at jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java:85. It needs a monitored VM to retrieve a main class name ( line 86) and use it on lines 100 and 104 to filter out processes that don't match.

private static boolean check(VirtualMachineDescriptor vmd, String excludeClass, String partialMatch) {
81 String mainClass = null;
82 try {
83 VmIdentifier vmId = new VmIdentifier(vmd.id());
84 MonitoredHost monitoredHost = MonitoredHost.getMonitoredHost(vmId);
85 MonitoredVm monitoredVm = monitoredHost.getMonitoredVm(vmId, -1);
86 mainClass = MonitoredVmUtil.mainClass(monitoredVm, true);
87 monitoredHost.detach(monitoredVm);
88 } catch (NullPointerException npe) {
89 // There is a potential race, where a running java app is being
90 // queried, unfortunately the java app has shutdown after this
91 // method is started but before getMonitoredVM is called.
92 // If this is the case, then the /tmp/hsperfdata_xxx/pid file
93 // will have disappeared and we will get a NullPointerException.
94 // Handle this gracefully....
95 return false;
96 } catch (MonitorException | URISyntaxException e) {
97 return false;
98 }
99
100 if (excludeClass != null && mainClass.equals(excludeClass)) {
101 return false;
102 }
103
104 if (partialMatch != null && mainClass.indexOf(partialMatch) == -1) {
105 return false;
106 }
107
108 return true;
109 }

Below is the stack trace for that:
attach:-1, Perf (jdk.internal.perf)
attachImpl:270, Perf (jdk.internal.perf)
attach:200, Perf (jdk.internal.perf)
<init>:64, PerfDataBuffer (sun.jvmstat.perfdata.monitor.protocol.local)
<init>:68, LocalMonitoredVm (sun.jvmstat.perfdata.monitor.protocol.local)
getMonitoredVm:77, MonitoredHostProvider (sun.jvmstat.perfdata.monitor.protocol.local)
check:85, ProcessArgumentMatcher (sun.tools.common)
getVMDs:129, ProcessArgumentMatcher (sun.tools.common)
getVirtualMachinePids:154, ProcessArgumentMatcher (sun.tools.common)
main:83, JCmd (sun.tools.jcmd)

Best regards,
Daniil

On 11/29/18, 5:16 PM, "Chris Plummer" <***@oracle.com> wrote:

Can you remind me why the test is trying to attach to all running java
processes.

Chris

Chris Plummer

2018-11-30 03:06:55 UTC

Permalink

Ok. So this is when you attach using a main classname rather than a pid.
We could probably make this test use a pid instead, but that doesn't
actually fix the real issue.

Chris

Post by Daniil Titov
Hi Chris,
It attaches to the instrumentation buffer when it is getting a MonitoredVm object inside check() method at jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java:85. It needs a monitored VM to retrieve a main class name ( line 86) and use it on lines 100 and 104 to filter out processes that don't match.
private static boolean check(VirtualMachineDescriptor vmd, String excludeClass, String partialMatch) {
81 String mainClass = null;
82 try {
83 VmIdentifier vmId = new VmIdentifier(vmd.id());
84 MonitoredHost monitoredHost = MonitoredHost.getMonitoredHost(vmId);
85 MonitoredVm monitoredVm = monitoredHost.getMonitoredVm(vmId, -1);
86 mainClass = MonitoredVmUtil.mainClass(monitoredVm, true);
87 monitoredHost.detach(monitoredVm);
88 } catch (NullPointerException npe) {
89 // There is a potential race, where a running java app is being
90 // queried, unfortunately the java app has shutdown after this
91 // method is started but before getMonitoredVM is called.
92 // If this is the case, then the /tmp/hsperfdata_xxx/pid file
93 // will have disappeared and we will get a NullPointerException.
94 // Handle this gracefully....
95 return false;
96 } catch (MonitorException | URISyntaxException e) {
97 return false;
98 }
99
100 if (excludeClass != null && mainClass.equals(excludeClass)) {
101 return false;
102 }
103
104 if (partialMatch != null && mainClass.indexOf(partialMatch) == -1) {
105 return false;
106 }
107
108 return true;
109 }
attach:-1, Perf (jdk.internal.perf)
attachImpl:270, Perf (jdk.internal.perf)
attach:200, Perf (jdk.internal.perf)
<init>:64, PerfDataBuffer (sun.jvmstat.perfdata.monitor.protocol.local)
<init>:68, LocalMonitoredVm (sun.jvmstat.perfdata.monitor.protocol.local)
getMonitoredVm:77, MonitoredHostProvider (sun.jvmstat.perfdata.monitor.protocol.local)
check:85, ProcessArgumentMatcher (sun.tools.common)
getVMDs:129, ProcessArgumentMatcher (sun.tools.common)
getVirtualMachinePids:154, ProcessArgumentMatcher (sun.tools.common)
main:83, JCmd (sun.tools.jcmd)
Best regards,
Daniil
Can you remind me why the test is trying to attach to all running java
processes.
Chris