[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: Fix race condition in jdwp
From:       Andrew Leonard <andrew_m_leonard () uk ! ibm ! com>
Date:       2018-04-11 13:33:00
Message-ID: OFDE16432A.2BD1B2D2-ON8025826C.0049AD1C-8025826C.004A7216 () notes ! na ! collabserv ! com
[Download RAW message or body]

This is a multipart message in MIME format.
--=_alternative 004A719F8025826C_=
Content-Type: text/plain; charset="US-ASCII"

Hi Serguei,
Thank you for raising the bug.
I had a chat with one of my colleagues who could recreate it, and it's 
probably related to the handshaking that is done in the particular 
scenario. So with the JCK harness:

com.sun.jck.lib.ExecJCKTestOtherJVMCmd LD_LIBRARY_PATH=/javatest/lib/jck
/jck8b/natives/linux_x86-64 /projects/jck/jdwp/j2sdk-image/bin/java 
-Xdump:system:none -Xdump:system:events=gpf+abort+traceassert+corruptcache 
-Xdump:snap:none -Xdump:snap:events=gpf+abort+traceassert+corruptcache 
-Xdump:java:none -Xdump:java:events=gpf+abort+traceassert+corruptcache 
-Xdump:heap:none -Xdump:heap:events=gpf+abort+traceassert+corruptcache -
Xfuture -agentlib:jdwp=server=y,transport=dt_socket,address=localhost
> 35000,suspend=y -classpath /javatest/lib/jck
/JCK8b-b03/JCK-runtime-8b/classes -Djava.security.policy=/javatest/lib/jck
/JCK8b-b03/JCK-runtime-8b/lib/jck.policy 
javasoft.sqe.jck.lib.jpda.jdwp.DebuggeeLoader -waittime=600 
-msgSwitch=ub1604x64vm10:38636 -componentName=
ArrayReference.GetValues.getvalues002

Note that the JCK test harness starts the target process, attaches to it, 
and sends the resume command 
in a very short time with no handshaking.

That may not help..but hopefully helps explain things a bit? It's the 
timing of the resume command during the test that is crucial, resuming 
before the VM initialization is complete will trigger it.

Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leonard@uk.ibm.com 




From:   "serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com>
To:     Andrew Leonard <andrew_m_leonard@uk.ibm.com>
Cc:     serviceability-dev@openjdk.java.net
Date:   11/04/2018 09:57
Subject:        Re: RFR: Fix race condition in jdwp



Hi Andrew,

I've filed the bug:
  https://bugs.openjdk.java.net/browse/JDK-8201409

Also, this is a webrev with your patch:
  
http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8201409-jdwp-initsync.ibm.1/


I agree that creating a standalone test is tricky here.

I've added usleep(10000) into the eventHelper_reportVMInit()
and ran the JTreg com/sun/jdi tests with my JDK build.
However, none of the tests failed with the failure mode you described.
So that I'm puzzled a little bit.
I suspect that some specific debugLoop commands were used in your 
scenario.

It is still possible that I've missed something here.
Will try to double check everything.

Thanks,
Serguei


On 4/11/18 01:29, Andrew Leonard wrote:
Thanks Serguei, 
I terms of a standalone testcase it is quite tricky, as due to the nature 
of the issue which took a lot of investigation to solve it's very timing 
dependent and will only occur randomly. It can be forced as I indicated 
below by adding a "sleep" in the VMInit report code but that's not a 
testcase, however the issue was originally found in our JCK testing for 
IBMJava8, testcase test.jck8b.runtime.vm.jdwp, but again only happened 
intermittently. Sort of like "performance" type issues we're not always 
going to be able to create a testcase that will always "fail" if the fix 
is not present. 
Your thoughts? 
Cheers 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leonard@uk.ibm.com 




From:        "serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com> 
To:        Andrew Leonard <andrew_m_leonard@uk.ibm.com> 
Cc:        serviceability-dev@openjdk.java.net 
Date:        11/04/2018 01:02 
Subject:        Re: RFR: Fix race condition in jdwp 



Hi Andrew,

Okay, I'll file a bug on this topic.
But do you have a standalone test demonstrating this issue?

Thanks,
Serguei


On 4/10/18 06:23, Andrew Leonard wrote: 
Hi Serguei, 
I don't have access to the bug database to raise one, are you able to 
please? 

Summary: JDWP debugger initialization hangs intermittently 
Description: If during the JDWP setup initialization the VM initialization 
takes slightly longer than the main debug initialization thread a "hang" 
situation can occur. This has been seen in testcase 
test.jck8b.runtime.vm.jdwp and can also be recreated easily by adding a 10 
second sleep to the beginning of the 
src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c method 
eventHelper_reportVMInit() . 
First seen: JDK8 
Recreated: JDK11 

Thanks 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leonard@uk.ibm.com 




From:        "serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com> 
To:        Andrew Leonard <andrew_m_leonard@uk.ibm.com>, 
serviceability-dev@openjdk.java.net 
Date:        09/04/2018 23:03 
Subject:        Re: RFR: Fix race condition in jdwp 



Hi Andrew,

The patch itself looks reasonable.
However, in order to proceed with it, a bug report with a standalone
test case demonstrating the issue is needed.

Thanks,
Serguei


On 4/9/18 09:07, Andrew Leonard wrote:
> Hi,
> We discovered in our testing with OpenJ9 that a race condition can 
> occur in the jdwp under certain circumstances, and we were able to 
> force the same issue with Hotspot. Normally, the event helper thread 
> suspends all threads, then the debug loop in the listener thread 
> receives a command to resume. The debugger may deadlock if the debug 
> loop in the listener thread starts processing commands (e.g. resume 
> threads) before the event helper completes the initialization (and 
> suspends threads).
> 
> This patch adds synchronization to ensure the event helper completes 
> the initialization sequence before debugger commands are processed.
> 
> Please can I find a sponsor for this contribution? Patch below..
> 
> Many thanks
> 
> Andrew
> 
> 
> 
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c 
> b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights 
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -58,6 +58,7 @@
> static jboolean vmInitialized;
> static jrawMonitorID initMonitor;
> static jboolean initComplete;
> +static jboolean VMInitComplete;
> static jbyte currentSessionID;
> 
> /*
> @@ -617,6 +618,35 @@
> debugMonitorExit(initMonitor);
> }
> 
> +/*
> + * Signal VM initialization is complete.
> + */
> +void
> +signalVMInitComplete(void)
> +{
> +    /*
> + * VM Initialization is complete
> + */
> +    LOG_MISC(("signal VM initialization complete"));
> +    debugMonitorEnter(initMonitor);
> +    VMInitComplete = JNI_TRUE;
> +    debugMonitorNotifyAll(initMonitor);
> +    debugMonitorExit(initMonitor);
> +}
> +
> +/*
> + * Wait for VM initialization to complete.
> + */
> +void
> +debugInit_waitVMInitComplete(void)
> +{
> +    debugMonitorEnter(initMonitor);
> +    while (!VMInitComplete) {
> +    debugMonitorWait(initMonitor);
> +    }
> +    debugMonitorExit(initMonitor);
> +}
> +
> /* All process exit() calls come from here */
> void
> forceExit(int exit_code)
> @@ -672,6 +702,7 @@
> LOG_MISC(("Begin initialize()"));
> currentSessionID = 0;
> initComplete = JNI_FALSE;
> +    VMInitComplete = JNI_FALSE;
> 
> if ( gdata->vmDead ) {
> EXIT_ERROR(AGENT_ERROR_INTERNAL,"VM dead at initialize() time");
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h 
> b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights 
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -39,4 +39,7 @@
> void debugInit_exit(jvmtiError, const char *);
> void forceExit(int);
> 
> +void debugInit_waitVMInitComplete(void);
> +void signalVMInitComplete(void);
> +
> #endif
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c 
> b/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights 
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -98,6 +98,7 @@
> standardHandlers_onConnect();
> threadControl_onConnect();
> 
> +    debugInit_waitVMInitComplete();
> /* Okay, start reading cmds! */
> while (shouldListen) {
> if (!dequeue(&p)) {
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c 
> b/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights 
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -580,6 +580,7 @@
> (void)threadControl_suspendThread(command->thread, JNI_FALSE);
> }
> 
> +    signalVMInitComplete();
> outStream_initCommand(&out, uniqueID(), 0x0,
> JDWP_COMMAND_SET(Event),
> JDWP_COMMAND(Event, Composite));
> 
> 
> 
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leonard@uk.ibm.com
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with 
> number 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU 





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU




Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

--=_alternative 004A719F8025826C_=
Content-Type: text/html; charset="US-ASCII"

<span style=" font-size:10pt;font-family:sans-serif">Hi Serguei,</span>
<br><span style=" font-size:10pt;font-family:sans-serif">Thank you for
raising the bug.</span>
<br><span style=" font-size:10pt;font-family:sans-serif">I had a chat with
one of my colleagues who could recreate it, and it's probably related to
the handshaking that is done in the particular scenario. So with the JCK
harness:</span>
<br>
<br><span style=" font-size:11pt;font-family:.SF NS \
Text">com.sun.jck.lib.ExecJCKTestOtherJVMCmd \
                LD_LIBRARY_PATH=/<u>javatest</u>/<u>lib</u>/<u>jck</u>/jck8b/natives/linux_x86-64
                
/projects/<u>jck</u>/jdwp/j2sdk-image/bin/java -Xdump:system:none \
                -Xdump:system:events=<u>gpf</u>+abort+<u>traceassert</u>+<u>corruptcache</u>
                
-Xdump:snap:none -Xdump:snap:events=<u>gpf</u>+abort+<u>traceassert</u>+<u>corruptcache</u>
                
-Xdump:java:none -Xdump:java:events=<u>gpf</u>+abort+<u>traceassert</u>+<u>corruptcache</u>
                
-Xdump:heap:none -Xdump:heap:events=<u>gpf</u>+abort+<u>traceassert</u>+<u>corruptcache</u>
                
-<u>Xfuture</u> -agentlib:jdwp=server=y,transport=dt_socket,address=<u>localhost</u>:35000,suspend=y
                
-<u>classpath</u> /<u>javatest</u>/<u>lib</u>/<u>jck</u>/JCK8b-b03/JCK-runtime-8b/classes
                
-Djava.security.policy=/<u>javatest</u>/<u>lib</u>/<u>jck</u>/JCK8b-b03/JCK-runtime-8b/<u>lib</u>/jck.policy
 javasoft.sqe.jck.lib.jpda.jdwp.DebuggeeLoader -<u>waittime</u>=600 \
                -msgSwitch=ub1604x64vm10:38636
-componentName=<b>ArrayReference.GetValues.getvalues002</b></span>
<br>
<br><span style=" font-size:11pt;font-family:.SF NS Text">Note that the
JCK test harness starts the target process, attaches to it, and sends the
resume command </span>
<br><span style=" font-size:11pt;font-family:.SF NS Text">in a very short
time with no handshaking.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">That may not help..but
hopefully helps explain things a bit? It's the timing of the resume command
during the test that is crucial, resuming before the VM initialization
is complete will trigger it.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">Thanks</span>
<br><span style=" font-size:10pt;font-family:sans-serif">Andrew</span>
<br>
<br><span style=" font-size:12pt">Andrew Leonard<br>
Java Runtimes Development<br>
IBM Hursley<br>
IBM United Kingdom Ltd<br>
Phone internal: 245913, external: 01962 815913<br>
internet email: andrew_m_leonard@uk.ibm.com </span>
<br>
<br>
<br>
<br>
<br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">From:
&nbsp; &nbsp; &nbsp; &nbsp;</span><span style=" \
font-size:9pt;font-family:sans-serif">&quot;serguei.spitsyn@oracle.com&quot; \
&lt;serguei.spitsyn@oracle.com&gt;</span> <br><span style=" \
font-size:9pt;color:#5f5f5f;font-family:sans-serif">To: &nbsp; &nbsp; &nbsp; \
&nbsp;</span><span style=" font-size:9pt;font-family:sans-serif">Andrew Leonard \
&lt;andrew_m_leonard@uk.ibm.com&gt;</span> <br><span style=" \
font-size:9pt;color:#5f5f5f;font-family:sans-serif">Cc: &nbsp; &nbsp; &nbsp; \
&nbsp;</span><span style=" \
font-size:9pt;font-family:sans-serif">serviceability-dev@openjdk.java.net</span> \
<br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">Date: &nbsp; \
&nbsp; &nbsp; &nbsp;</span><span style=" \
font-size:9pt;font-family:sans-serif">11/04/2018 09:57</span>
<br><span style=" font-size:9pt;color:#5f5f5f;font-family:sans-serif">Subject:
&nbsp; &nbsp; &nbsp; &nbsp;</span><span style=" \
                font-size:9pt;font-family:sans-serif">Re:
RFR: Fix race condition in jdwp</span>
<br>
<hr noshade>
<br>
<br>
<br><span style=" font-size:12pt">Hi Andrew,<br>
<br>
I've filed the bug:<br>
&nbsp; </span><a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openj \
dk.java.net_browse_JDK-2D8201409&amp;d=DwMDaQ&amp;c=jf_iaSHvJObTbx-siA1ZOg&amp;r=NaV8I \
y8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ&amp;m=nYdDNT1pdP7gQK1JviPJ6zIxLWcwe7R-azh_NnvI0Ok&amp;s=z0_3fj5wG3cofrwXExX0jmnKvUeBGxjAOsYE1Gz6xCg&amp;e="><span \
style=" font-size:12pt;color:blue"><u>https://bugs.openjdk.java.net/browse/JDK-8201409</u></span></a><span \
style=" font-size:12pt"><br> <br>
Also, this is a webrev with your patch:<br>
&nbsp; </span><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk. \
java.net_-7Esspitsyn_webrevs_2018_8201409-2Djdwp-2Dinitsync.ibm.1_&amp;d=DwMDaQ&amp;c= \
jf_iaSHvJObTbx-siA1ZOg&amp;r=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ&amp;m=nYdDNT1 \
pdP7gQK1JviPJ6zIxLWcwe7R-azh_NnvI0Ok&amp;s=8LH2-PYUqJbVkawhCgmuD5106lOuFKI-jAVhCpB_tYY&amp;e="><span \
style=" font-size:12pt;color:blue"><u>http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8201409-jdwp-initsync.ibm.1/</u></span></a><span \
style=" font-size:12pt"><br> <br>
I agree that creating a standalone test is tricky here.<br>
<br>
I've added usleep(10000) into the eventHelper_reportVMInit()<br>
and ran the JTreg com/sun/jdi tests with my JDK build.<br>
However, none of the tests failed with the failure mode you described.<br>
So that I'm puzzled a little bit.<br>
I suspect that some specific debugLoop commands were used in your scenario.<br>
<br>
It is still possible that I've missed something here.<br>
Will try to double check everything.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 4/11/18 01:29, Andrew Leonard wrote:</span>
<br><span style=" font-size:12pt">Thanks Serguei, <br>
I terms of a standalone testcase it is quite tricky, as due to the nature
of the issue which took a lot of investigation to solve it's very timing
dependent and will only occur randomly. It can be forced as I indicated
below by adding a &quot;sleep&quot; in the VMInit report code but that's
not a testcase, however the issue was originally found in our JCK testing
for IBMJava8, testcase test.jck8b.runtime.vm.jdwp, but again only happened
intermittently. Sort of like &quot;performance&quot; type issues we're
not always going to be able to create a testcase that will always &quot;fail&quot;
if the fix is not present. <br>
Your thoughts? <br>
Cheers <br>
Andrew <br>
<br>
Andrew Leonard<br>
Java Runtimes Development<br>
IBM Hursley<br>
IBM United Kingdom Ltd<br>
Phone internal: 245913, external: 01962 815913<br>
internet email: </span><a href=mailto:andrew_m_leonard@uk.ibm.com><span style=" \
font-size:12pt;color:blue"><u>andrew_m_leonard@uk.ibm.com</u></span></a><span style=" \
font-size:12pt"> <br>
<br>
<br>
<br>
<br>
From: &nbsp; &nbsp; &nbsp; &nbsp;</span><a \
href=mailto:serguei.spitsyn@oracle.com><span style=" \
font-size:12pt;color:blue"><u>&quot;serguei.spitsyn@oracle.com&quot;</u></span></a><span \
style=" font-size:12pt"> </span><a href=mailto:serguei.spitsyn@oracle.com><span \
style=" font-size:12pt;color:blue"><u>&lt;serguei.spitsyn@oracle.com&gt;</u></span></a><span \
style=" font-size:12pt"> <br>
To: &nbsp; &nbsp; &nbsp; &nbsp;Andrew Leonard </span><a \
href=mailto:andrew_m_leonard@uk.ibm.com><span style=" \
font-size:12pt;color:blue"><u>&lt;andrew_m_leonard@uk.ibm.com&gt;</u></span></a><span \
style=" font-size:12pt"> <br>
Cc: &nbsp; &nbsp; &nbsp; &nbsp;</span><a \
href="mailto:serviceability-dev@openjdk.java.net"><span style=" \
font-size:12pt;color:blue"><u>serviceability-dev@openjdk.java.net</u></span></a><span \
style=" font-size:12pt"> <br>
Date: &nbsp; &nbsp; &nbsp; &nbsp;11/04/2018 01:02 <br>
Subject: &nbsp; &nbsp; &nbsp; &nbsp;Re: RFR: Fix race condition in jdwp
<br>
</span>
<hr><span style=" font-size:12pt"><br>
<br>
<br>
Hi Andrew,<br>
<br>
Okay, I'll file a bug on this topic.<br>
But do you have a standalone test demonstrating this issue?<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 4/10/18 06:23, Andrew Leonard wrote: <br>
Hi Serguei, <br>
I don't have access to the bug database to raise one, are you able to please?
<br>
<br>
Summary: JDWP debugger initialization hangs intermittently <br>
Description: If during the JDWP setup initialization the VM initialization
takes slightly longer than the main debug initialization thread a &quot;hang&quot;
situation can occur. This has been seen in testcase test.jck8b.runtime.vm.jdwp
and can also be recreated easily by adding a 10 second sleep to the beginning
of the src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c method \
                eventHelper_reportVMInit()
. <br>
First seen: JDK8 <br>
Recreated: JDK11 <br>
<br>
Thanks <br>
Andrew <br>
<br>
Andrew Leonard<br>
Java Runtimes Development<br>
IBM Hursley<br>
IBM United Kingdom Ltd<br>
Phone internal: 245913, external: 01962 815913<br>
internet email: </span><a href=mailto:andrew_m_leonard@uk.ibm.com><span style=" \
font-size:12pt;color:blue"><u>andrew_m_leonard@uk.ibm.com</u></span></a><span style=" \
font-size:12pt"> <br>
<br>
<br>
<br>
<br>
From: &nbsp; &nbsp; &nbsp; &nbsp;</span><a \
href=mailto:serguei.spitsyn@oracle.com><span style=" \
font-size:12pt;color:blue"><u>&quot;serguei.spitsyn@oracle.com&quot;</u></span></a><span \
style=" font-size:12pt"> </span><a href=mailto:serguei.spitsyn@oracle.com><span \
style=" font-size:12pt;color:blue"><u>&lt;serguei.spitsyn@oracle.com&gt;</u></span></a><span \
style=" font-size:12pt"> <br>
To: &nbsp; &nbsp; &nbsp; &nbsp;Andrew Leonard </span><a \
href=mailto:andrew_m_leonard@uk.ibm.com><span style=" \
font-size:12pt;color:blue"><u>&lt;andrew_m_leonard@uk.ibm.com&gt;</u></span></a><span \
style=" font-size:12pt">, </span><a \
href="mailto:serviceability-dev@openjdk.java.net"><span style=" \
font-size:12pt;color:blue"><u>serviceability-dev@openjdk.java.net</u></span></a><span \
style=" font-size:12pt"> <br>
Date: &nbsp; &nbsp; &nbsp; &nbsp;09/04/2018 23:03 <br>
Subject: &nbsp; &nbsp; &nbsp; &nbsp;Re: RFR: Fix race condition in jdwp
<br>
</span>
<hr><span style=" font-size:12pt"><br>
</span><tt><span style=" font-size:12pt"><br>
<br>
Hi Andrew,<br>
<br>
The patch itself looks reasonable.<br>
However, in order to proceed with it, a bug report with a standalone<br>
test case demonstrating the issue is needed.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 4/9/18 09:07, Andrew Leonard wrote:<br>
&gt; Hi,<br>
&gt; We discovered in our testing with OpenJ9 that a race condition can
<br>
&gt; occur in the jdwp under certain circumstances, and we were able to
<br>
&gt; force the same issue with Hotspot. Normally, the event helper thread
<br>
&gt; suspends all threads, then the debug loop in the listener thread <br>
&gt; receives a command to resume. The debugger may deadlock if the debug
<br>
&gt; loop in the listener thread starts processing commands (e.g. resume
<br>
&gt; threads) before the event helper completes the initialization (and
<br>
&gt; suspends threads).<br>
&gt;<br>
&gt; This patch adds synchronization to ensure the event helper completes
<br>
&gt; the initialization sequence before debugger commands are processed.<br>
&gt;<br>
&gt; Please can I find a sponsor for this contribution? Patch below..<br>
&gt;<br>
&gt; Many thanks<br>
&gt;<br>
&gt; Andrew<br>
&gt;<br>
&gt;<br>
&gt;<br>
&gt; diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c <br>
&gt; b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c<br>
&gt; --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c<br>
&gt; +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c<br>
&gt; @@ -1,5 +1,5 @@<br>
&gt; &nbsp;/*<br>
&gt; - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; &nbsp; * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.<br>
&gt; &nbsp; *<br>
&gt; &nbsp; * This code is free software; you can redistribute it and/or
modify it<br>
&gt; @@ -58,6 +58,7 @@<br>
&gt; &nbsp;static jboolean vmInitialized;<br>
&gt; &nbsp;static jrawMonitorID initMonitor;<br>
&gt; &nbsp;static jboolean initComplete;<br>
&gt; +static jboolean VMInitComplete;<br>
&gt; &nbsp;static jbyte currentSessionID;<br>
&gt;<br>
&gt; &nbsp;/*<br>
&gt; @@ -617,6 +618,35 @@<br>
&gt; &nbsp;debugMonitorExit(initMonitor);<br>
&gt; &nbsp;}<br>
&gt;<br>
&gt; +/*<br>
&gt; + * Signal VM initialization is complete.<br>
&gt; + */<br>
&gt; +void<br>
&gt; +signalVMInitComplete(void)<br>
&gt; +{<br>
&gt; + &nbsp; &nbsp;/*<br>
&gt; + * VM Initialization is complete<br>
&gt; + */<br>
&gt; + &nbsp; &nbsp;LOG_MISC((&quot;signal VM initialization complete&quot;));<br>
&gt; + &nbsp; &nbsp;debugMonitorEnter(initMonitor);<br>
&gt; + &nbsp; &nbsp;VMInitComplete = JNI_TRUE;<br>
&gt; + &nbsp; &nbsp;debugMonitorNotifyAll(initMonitor);<br>
&gt; + &nbsp; &nbsp;debugMonitorExit(initMonitor);<br>
&gt; +}<br>
&gt; +<br>
&gt; +/*<br>
&gt; + * Wait for VM initialization to complete.<br>
&gt; + */<br>
&gt; +void<br>
&gt; +debugInit_waitVMInitComplete(void)<br>
&gt; +{<br>
&gt; + &nbsp; &nbsp;debugMonitorEnter(initMonitor);<br>
&gt; + &nbsp; &nbsp;while (!VMInitComplete) {<br>
&gt; + &nbsp; &nbsp;debugMonitorWait(initMonitor);<br>
&gt; + &nbsp; &nbsp;}<br>
&gt; + &nbsp; &nbsp;debugMonitorExit(initMonitor);<br>
&gt; +}<br>
&gt; +<br>
&gt; &nbsp;/* All process exit() calls come from here */<br>
&gt; &nbsp;void<br>
&gt; &nbsp;forceExit(int exit_code)<br>
&gt; @@ -672,6 +702,7 @@<br>
&gt; &nbsp;LOG_MISC((&quot;Begin initialize()&quot;));<br>
&gt; &nbsp;currentSessionID = 0;<br>
&gt; &nbsp;initComplete = JNI_FALSE;<br>
&gt; + &nbsp; &nbsp;VMInitComplete = JNI_FALSE;<br>
&gt;<br>
&gt; &nbsp;if ( gdata-&gt;vmDead ) {<br>
&gt; &nbsp; &nbsp; &nbsp;EXIT_ERROR(AGENT_ERROR_INTERNAL,&quot;VM dead
at initialize() time&quot;);<br>
&gt; diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h <br>
&gt; b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h<br>
&gt; --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h<br>
&gt; +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.h<br>
&gt; @@ -1,5 +1,5 @@<br>
&gt; &nbsp;/*<br>
&gt; - * Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; &nbsp; * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.<br>
&gt; &nbsp; *<br>
&gt; &nbsp; * This code is free software; you can redistribute it and/or
modify it<br>
&gt; @@ -39,4 +39,7 @@<br>
&gt; &nbsp;void debugInit_exit(jvmtiError, const char *);<br>
&gt; &nbsp;void forceExit(int);<br>
&gt;<br>
&gt; +void debugInit_waitVMInitComplete(void);<br>
&gt; +void signalVMInitComplete(void);<br>
&gt; +<br>
&gt; &nbsp;#endif<br>
&gt; diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c <br>
&gt; b/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c<br>
&gt; --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c<br>
&gt; +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c<br>
&gt; @@ -1,5 +1,5 @@<br>
&gt; &nbsp;/*<br>
&gt; - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; &nbsp; * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.<br>
&gt; &nbsp; *<br>
&gt; &nbsp; * This code is free software; you can redistribute it and/or
modify it<br>
&gt; @@ -98,6 +98,7 @@<br>
&gt; &nbsp;standardHandlers_onConnect();<br>
&gt; &nbsp;threadControl_onConnect();<br>
&gt;<br>
&gt; + &nbsp; &nbsp;debugInit_waitVMInitComplete();<br>
&gt; &nbsp;/* Okay, start reading cmds! */<br>
&gt; &nbsp;while (shouldListen) {<br>
&gt; &nbsp; &nbsp; &nbsp;if (!dequeue(&amp;p)) {<br>
&gt; diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
<br>
&gt; b/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c<br>
&gt; --- a/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c<br>
&gt; +++ b/src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c<br>
&gt; @@ -1,5 +1,5 @@<br>
&gt; &nbsp;/*<br>
&gt; - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights
<br>
&gt; reserved.<br>
&gt; &nbsp; * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.<br>
&gt; &nbsp; *<br>
&gt; &nbsp; * This code is free software; you can redistribute it and/or
modify it<br>
&gt; @@ -580,6 +580,7 @@<br>
&gt; &nbsp; &nbsp; &nbsp;(void)threadControl_suspendThread(command-&gt;thread,
JNI_FALSE);<br>
&gt; &nbsp;}<br>
&gt;<br>
&gt; + &nbsp; &nbsp;signalVMInitComplete();<br>
&gt; &nbsp;outStream_initCommand(&amp;out, uniqueID(), 0x0,<br>
&gt; &nbsp;JDWP_COMMAND_SET(Event),<br>
&gt; &nbsp;JDWP_COMMAND(Event, Composite));<br>
&gt;<br>
&gt;<br>
&gt;<br>
&gt; Andrew Leonard<br>
&gt; Java Runtimes Development<br>
&gt; IBM Hursley<br>
&gt; IBM United Kingdom Ltd<br>
&gt; Phone internal: 245913, external: 01962 815913<br>
&gt; internet email: </span></tt><a href=mailto:andrew_m_leonard@uk.ibm.com><tt><span \
style=" font-size:12pt;color:blue"><u>andrew_m_leonard@uk.ibm.com</u></span></tt></a><tt><span \
style=" font-size:12pt"><br> &gt;<br>
&gt;<br>
&gt; Unless stated otherwise above:<br>
&gt; IBM United Kingdom Limited - Registered in England and Wales with
<br>
&gt; number 741598.<br>
&gt; Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU</span></tt><span style=" font-size:12pt"><br>
<br>
<br>
<br>
<br>
<br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU <br>
<br>
<br>
<br>
<br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU</span>
<br>
<br>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif"><br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU<br>
</span>
--=_alternative 004A719F8025826C_=--


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic