On Tue, 22 Sep 2026 07:19:17 GMT, David Holmes <[email protected]> wrote:
>> The `GetThreadSnapshotHandshakeClosure` was added to support virtual threads >> in thread dumps. It is very similar to the logic used in the old >> safepoint-based thread dump. >> `GetThreadSnapshotHandshakeClosure::detect_locks` uses the raw >> `javaVFrame::monitors()` method rather than using the `locked_monitors()` >> method which already filters out some monitors including those for which >> `wait()` has been called. It uses the raw `monitors()` list because it wants >> to process eliminated compiled monitors itself, and they are already removed >> from `locked_monitors()`. But that means it should be doing its own >> filtering of monitors that are being waited-on so they are not reported as >> locked. This seems to have been an oversight with the original >> implementation. >> >> The logic is somewhat more complicated, particularly in the virtual thread >> case, because there are a number of points where we can dump the stack >> between `wait0` appearing as the top frame, and the point where we actually >> release the monitor. It would be wrong to hide the monitor from the "locked" >> section in that case. So we can still report that a thread is waiting-on a >> particular object and that it has locked that object, as that is the actual >> state of things. But the code no longer reports a released monitor as locked >> by the waiting thread. >> >> Note that `javaVFrame::locked_monitors` is also potentially imprecise in its >> reporting. It will report the monitor as locked until >> `current_waiting_monitor()` is set, and then elide it. However, as this code >> is only used when reporting platform threads this imprecision is not >> observable as a platform thread will not respond to a stack dump request in >> between the transition to in-VM (for the native call) and the release of the >> monitor. >> >> I also fixed a pre-existing typo whilst in this code. >> >> Testing >> - tiers 1-3 >> - tier 5-svc >> - updated tests in the PR >> >> Thanks >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > David Holmes has updated the pull request incrementally with one additional > commit since the last revision: > > Adjust test per Alan's request test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java line 370: > 368: // then we may not have the object that the thread is > waiting on. > 369: Method wait0 = Object.class.getDeclaredMethod("wait0", > long.class); > 370: boolean expectWaitingOn = > !WhiteBox.getWhiteBox().isMethodCompiled(wait0); Now I'm wondering if expectWaitingOn is needed, and if we can checking waitingOn unconditionally. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32977#discussion_r4070773341
