Hi David, The existing "initComplete" in debugInit.c logic is the debug main thread initialization complete logic, that does not handle nor needs to wait for the asynchronous VM initialization that will be reported but must be completed before the debug loop can start processing cmds. Thanks Andrew
Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: David Holmes <david.hol...@oracle.com> To: Andrew Leonard <andrew_m_leon...@uk.ibm.com>, serviceability-dev@openjdk.java.net Date: 10/04/2018 02:06 Subject: Re: RFR: Fix race condition in jdwp Hi Andrew, On 10/04/2018 2:07 AM, 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. How does this relate to the existing debugInit_waitInitComplete? I don't see why we would want two initialization synchronization mechanisms. ?? Thanks, David > 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_leon...@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