On Fri, 22 Oct 2021 06:21:37 GMT, Richard Reingruber <rr...@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. Hi Richard, The fix looks okay to me. I've inlined one minor suggestion. Thanks, Serguei src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 2194: > 2192: debugMonitorExit(threadLock); > 2193: eventHandler_lock(); > 2194: debugMonitorEnter(threadLock); The lines 2192-2194 look like a hack to me. But I see you have filed an enhancement to fix this. I'm okay to address it later. ------------- Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5849
