On Wed, 29 Jul 2026 21:08:42 GMT, Coleen Phillimore <[email protected]> wrote:
> This fixes the crash when printing deadlocks when the only lock held for the > thread in the deadlock is the JVMTI raw monitor lock. This also fixes > deadlock detection for only raw monitor locks and adds a test for it. > > make test TEST="serviceability/dcmd/thread > vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock > serviceability/jvmti/SuspendWithRawMonitorEnter > serviceability/jvmti/vthread/RawMonitorTest" > > Tested with jvmti deadlock detection tests above, and tier 1-4 in progress. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Looks good. The basic missing null check was easy to reason about - the rest took a bit more effort. I have a few minor nits with the test. Thanks src/hotspot/share/services/threadService.cpp line 520: > 518: previousThread = currentThread; > 519: waitingToLockMonitor = > (ObjectMonitor*)currentThread->current_pending_monitor(); > 520: waitingToLockRawMonitor = > currentThread->current_pending_raw_monitor(); This seems reasonable, but I do worry a little because of the comment: // JVM TI raw monitors can also be involved in deadlocks, and we can be // waiting to lock both a raw monitor and ObjectMonitor at the same time. // It isn't clear how to make deadlock detection work correctly if that // happens. Updating the raw monitor here might change something - but I struggle to see exactly how this cycle detection is working anyway. src/hotspot/share/services/threadService.cpp line 1054: > 1052: continue; > 1053: } > 1054: st->print_cr("%s \"%s\"", owner_desc, currentThread->name()); I was puzzled why you moved and duplicated this, but if I am reading things correctly the old code would cause two similar print outs for the raw monitor case. test/hotspot/jtreg/serviceability/dcmd/thread/PrintRawMonitorLockTest.java line 41: > 39: * @test > 40: * @bug 8253442 > 41: * @summary Test of diagnostic command Thread.print with only JVMTI raw > monitor. That doesn't seem to summarise this test test/hotspot/jtreg/serviceability/dcmd/thread/PrintRawMonitorLockTest.java line 53: > 51: > 52: static private void log(String s) { System.out.println(s); } > 53: static private String AGENT_LIB = "PrintRawMonitorLockTest";; Suggestion: private static void log(String s) { System.out.println(s); } private static String AGENT_LIB = "PrintRawMonitorLockTest";; test/hotspot/jtreg/serviceability/dcmd/thread/PrintRawMonitorLockTest.java line 56: > 54: > 55: native static int createRawMonitors(); > 56: native static int rawMonitorEnter(int id); Suggestion: static native int createRawMonitors(); static native int rawMonitorEnter(int id); test/hotspot/jtreg/serviceability/dcmd/thread/PrintRawMonitorLockTest.java line 97: > 95: log("entered lock1"); > 96: > 97: /* Hold lock on "lock" to show up in thread dump */ Comment from an older version? The name "lock" is not used in the test. test/hotspot/jtreg/serviceability/dcmd/thread/PrintRawMonitorLockTest.java line 106: > 104: "retCode=" + retCode); > 105: } > 106: log("tried to enter lock2"); If you get here you succeeded. ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/32092#pullrequestreview-4814447187 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679276998 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679281308 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679316799 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679314524 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679312229 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679324599 PR Review Comment: https://git.openjdk.org/jdk/pull/32092#discussion_r3679325806
