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

[Ralf Schmelter]

On Fri, 22 Oct 2021 06:21:37 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> This change fixes deadlocks described in the JBS-bug by:
>> 
>> * Releasing `handlerLock` before waiting on `threadLock` in `blockOnDebuggerSuspend()`
>> 
>> * Notifying on `threadLock` in `threadControl_reset()`
>> 
>> Also the actions in handleAppResumeBreakpoint() are moved/deferred until
>> doPendingTasks() runs. This is necessary because an event handler must not
>> release handlerLock first and foremost because handlers are called while
>> iterating the handler chain for an event type which is protected by handlerLock
>> (see https://github.com/openjdk/jdk/pull/5805)
>> 
>> The first commit delays the cleanup actions after leaving the loop in
>> `debugLoop_run()`. It allows to reproduce the deadlock running the dispose003
>> test with the command
>> 
>> 
>> make run-test TEST=test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003
>> 
>> 
>> The second commit adds a new test that reproduces the deadlock when calling
>> threadControl_resumeThread() while a thread is waiting in
>> blockOnDebuggerSuspend().
>> 
>> The third commit contains the fix described above. With it the deadlocks
>> cannot be reproduced anymore.
>> 
>> The forth commit removes the diagnostic code introduced with the first commit again.
>> 
>> The fix passed
>> 
>> test/hotspot/jtreg/serviceability/jdwp
>> test/jdk/com/sun/jdi
>> test/hotspot/jtreg/vmTestbase/nsk/jdwp
>> test/hotspot/jtreg/vmTestbase/nsk/jdi
>
> Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improve comment as suggested by Chris.

LGTM

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 812:

> 810: {
> 811:     jthread resumer = evinfo->thread;
> 812:     ThreadNode *node;

You could move the declaration into the if() block below, since it is not needed elsewhere.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 2192:

> 2190:          * ordering handlerLock has to be acquired before threadLock.
> 2191:          */
> 2192:         debugMonitorExit(threadLock);

You could move this to the if (resumer != NULL) block, since otherwise all the locking and unlocking is not needed anyways as far as I can see.

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

Marked as reviewed by rschmelter (Reviewer).

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