RFR: Fix race condition in jdwp

[David Holmes]

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_leonard at 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