On Mon, 6 May 2024 21:22:05 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> This PR adds ranked monitor support to the debug agent. The debug agent has >> a large number of monitors, and it's really hard to know which order to grab >> them in, and for that matter which monitors might already be held at any >> given moment. By imposing a rank on each monitor, we can check to make sure >> they are always grabbed in the order of their rank. Having this in place >> when I was working on >> [JDK-8324868](https://bugs.openjdk.org/browse/JDK-8324868) would have made >> it much easier to detect a deadlock that was occuring, and the reason for >> it. That's what motivated me to do this work >> >> There were 2 or 3 minor rank issues discovered as a result of these changes. >> I also learned a lot about some of the more ugly details of the locking >> support in the process. >> >> Tested with the following on all supported platforms and with virtual >> threads: >> >> com/sun/jdi >> vmTestbase/nsk/jdi >> vmTestbase/nsk/jdb >> vmTestbase/nsk/jdwp >> >> Still need to run tier2 and tier5. >> >> Details of the changes follow in the first comment. > > Chris Plummer has updated the pull request incrementally with eight > additional commits since the last revision: > > - Minor comment fix in debugMonitorCreate() > - Don't do anything in assertIsCurrentThread() if asserts are disabled > - Print threads names instead of addresses in assertIsCurrentThread() > - Renamed thread -> savedOwnerThread and entryCount -> savedEntryCount. > - pass already fetched current_thread to assertIsCurrentThread() > - dbgRawMonitor reference should be dbg_monitor->monitor in dumpRawMonitors() > - Free localrefs allocated by GetThreadInfo(). > - Optimize loop iterations during rank verification. src/jdk.jdwp.agent/share/native/libjdwp/util.c line 1203: > 1201: > 1202: // Need to lock during initialization so verifyMonitorRank() can be > guaranteed that > 1203: // if the monitor field is not NULL, then the monitor if fully > initialized. Nit: It looks like a typo: "the monitor if fully" => "the monitor is fully" ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1591680028
