On Thu, 2 May 2024 20:58:27 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 656:
>>
>>> 654: commonRef_lock();
>>> 655: if (gdata->rankedMonitors) {
>>> 656: dbgRawMonitor_lock();
>>
>> What is this call for? I think dbgRawMonitor_lock/dbgRawMonitor_unlock
>> should not be used outside of util.c (I'd remove them from util.h)
>
> After calling getLocks(), there may still be attempts to enter locks. The
> locks should already be held. I filed
> [JDK-8330193](https://bugs.openjdk.org/browse/JDK-8330193) to assert that.
> However, even when entering an already entered lock, we still need to grab
> dbgRawMonitor. I found that out the hard way when there were deadlocks on
> dbgRawMonitor because it was not entered it here.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1605484995
