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

Alex Menkov amenkov at openjdk.org
Sat May 18 01:15:08 UTC 2024


On Fri, 17 May 2024 22:56:25 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> I think expose dbgRawMonitor outside of util.c is wrong (and use gdata->rankedMonitors outside of utils.c too).
>> If it's really required, it would be better to add functions to disable/enable monitor locks.
>> But I still don't understand why we need this (and what is JDK-8330193 about)
>> Both JVMTI raw monitors and DebugRawMonitor support re-entrance, so this thread may enter the locks again (and other threads will wait for the locks if they try to enter them).
>> And I don't see how we can get deadlocks on dbgRawMonitor, could you please elaborate on that.
>
> The comment above getLocks() pretty much explains it. Before doing any thread suspension we can't allow any other thread to hold a lock that might be needed during suspension. This is why getLocks() is used to grab all these locks in advanced. If they were not grabbed in advanced and one of the locks was held by another thread that got suspended, then eventually the current thread (the one that initiated the thread suspension) would block on the suspended thread, which means it would never get resumed, so we have a deadlock. If we don't include dbgRawMonitor in this set of locks and we suspend a thread while it holds it, this prevents the current thread from doing a debugMonitorEnter on any lock, even ones it already holds. Note we can't check if the current thread owns a lock without grabbing dbgRawMonitor. In fact the main purpose of dbgRawMonitor is to allow the current thread to check if it owns the monitor.
> 
> I don't disagree that it's a bit of a wart that dbgRawMonitor is exposed in this manner. I just don't see a way around it. I can hide gdata->rankedMonitors in dbgRawMonitor_lock/unlock() if you want, but I can't see of way to not export dbgRawMonitor_lock/unlock() in some fashion, possibly with a different name.

Thank you for the explanation, I missed that threads can be suspended during checking for ownerThread.
With this lock no other thread (except current) can enter/exit/wait any monitor, but I suppose it's ok as dbgRawMonitor is locked only while current thread suspends required thread(s)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1605637099


More information about the serviceability-dev mailing list