On Tue, 22 Sep 2026 05:17:12 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
>>  - 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:
> 
>   Update the logic to check that the waiting thread has actually released the 
> monitor.
>   Update test accordingly to force release.x

.

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

Changes requested by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/32977#pullrequestreview-5274627074

Reply via email to