RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]

[Chris Plummer]

On Wed, 13 Oct 2021 13:24:06 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 2220:
>> 
>>> 2218:              * suspends a thread it will remain suspended.
>>> 2219:              */
>>> 2220:             trackAppResume(resumer);
>> 
>> `trackAppResume()` doesn't really need the handlerLock. However, it will grab it when calling `eventHandler_createInternalThreadOnly()`. Since you want to make sure it is grabbed before threadLock in order to preserve lock ordering, that complicates things if we decided not to grab the handlerLock in the code above. What I'm now thinking is all that is really needed is to hold the threadLock around the call to `blockOnDebuggerSuspend()`, or better yet just grab it from within `blockOnDebuggerSuspend()`. You probably don't need to do anything with handlerLock or threadLock inside of `doPendingTasks()`.
>
> After returning from `blockOnDebuggerSuspend()` we have to make sure resumee
> cannot be suspended by JDWP means otherwise the current thread which is about to
> execute j.l.Thread.resume() will interfere with the debugger. This is achieved
> by holding threadLock until the tracking is established by `trackAppResume()`.
> 
> For symmetry the set of owned locks should be equal before/after calling
> `blockOnDebuggerSuspend()` I think. Therefore handlerLock and threadLock are
> acquired before.

Ok, so you need to hold threadLock from the time `blockOnDebuggerSuspend()` is done waiting on it until after `trackAppResume()` is done, and since `trackAppResume()` needs to grab the handlerLock, and you need to grab the handerLock before the threadLock, you need to jump through some lock grabbing/release hoops. However, you are in fact releasing the threadLock for a short time in `blockOnDebuggerSuspend()` after the wait completes. Doesn't this expose the resumee to potential suspending after the wait has completed and before `trackAppResume()` has been called?

-------------

PR: https://git.openjdk.java.net/jdk/pull/5849