On Fri, 17 May 2024 22:56:25 GMT, Chris Plummer <cjplum...@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
