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

[Chris Plummer]

On Wed, 20 Oct 2021 06:50:51 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 770:
>> 
>>> 768:      * handlerLock which has to be acquired before threadLock.
>>> 769:      */
>>> 770:     debugMonitorExit(threadLock);
>> 
>> I think we can avoid the exit here. threadLock was grabbed in `threadControl_onEventHandlerExit()`. It probably should be released before calling `doPendingTasks()`. My suggestion would be to first release it right after the `findThread()` call, and then in the `ei == EI_THREAD_END` block grab it again at the start of the block and release at the end of the block.
>
> But fields of ThreadNode "node" (aka "resumer") are read and also modified in `doPendingTasks()` and also in `threadControl_onEventHandlerExit()`. IMHO this needs to be synchronized with threadLock. At least it is not obvious that the synchronization is not needed.

You already release threadLock temporarily in `trackAppResume()` and you used to release it temporarily in doPendingTasks(). I'm suggesting to instead release it in `threadControl_onEventHandlerExit()` before calling `doPendingTasks()` and then grab it again (after first grabbing the handlerLock) in `doPendingTasks()` before calling `trackAppResume()`. What can be modified in the ThreadNode during that time that you are concerned about? threadLock would still be held when `blockOnDebuggerSuspend()` is called.  The only interesting lines that are currently executed while holding the threadLock but no longer would be are:

2193     if (node->handlingAppResume) {
2194         jthread resumer = node->thread;
2195         jthread resumee = getResumee(resumer);

I don't think any of those can change, because only the currently executing thread can change them (when the Thread.resume() breakpoint is hit).

The suspendCount is only meaningful when in `blockOnDebuggerSuspend()`, and we'll be under the threadLock by that point.

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

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