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