RFR: Fix race condition in jdwp

[serguei.spitsyn at oracle.com]

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