RFR: 8328866: Add raw monitor rank support to the debug agent. [v2]

Serguei Spitsyn sspitsyn at openjdk.org
Tue May 7 00:35:53 UTC 2024


On Mon, 6 May 2024 21:22:05 GMT, Chris Plummer <cjplummer at 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


More information about the serviceability-dev mailing list