On Mon, 17 Nov 2025 13:50:30 GMT, Anton Artemov <[email protected]> wrote:

>> Hi, please consider the following changes:
>> 
>> If suspension is allowed when a thread is re-entering an object monitor 
>> (OM), then a deadlock is possible:
>> 
>> The waiting thread is made to be a successor and is unparked. Upon a 
>> suspension request, the thread will suspend itself whilst clearing the 
>> successor. The OM will be left unlocked (not grabbed by any thread), while 
>> the other threads are parked until a thread grabs the OM and the exits it. 
>> The suspended thread is on the entry-list and can be selected as a successor 
>> again. None of other threads can be woken up to grab the OM until the 
>> suspended thread has been resumed and successfully releases the OM.
>> 
>> This can happen in two places where the successor could be suspended: 
>> 1:
>> https://github.com/openjdk/jdk/blob/6322aaba63b235cb6c73d23a932210af318404ec/src/hotspot/share/runtime/objectMonitor.cpp#L1897
>> 
>> 2:
>> https://github.com/openjdk/jdk/blob/6322aaba63b235cb6c73d23a932210af318404ec/src/hotspot/share/runtime/objectMonitor.cpp#L1149
>> 
>> The issues are addressed by not allowing suspension in case 1, and by 
>> handling the suspension request at a later stage, after the thread has 
>> grabbed the OM in `reenter_internal()` in case 2. In case of a suspension 
>> request, the thread exits the OM and enters it again once resumed. 
>> 
>> The JVMTI `waited` event posting (2nd one) is postponed until the suspended 
>> thread is resumed and has entered the OM again.  The `enter` to the OM (in 
>> case `ExitOnSuspend` did exit) is done without posting any events.
>> 
>> Tests are added for both scenarios. 
>> 
>> Tested in tiers 1 - 7.
>
> Anton Artemov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8366659: Addressed reviewer's comments.

src/hotspot/share/runtime/objectMonitor.cpp line 1950:

> 1948:         // as having "-locked" the monitor, but the OS and 
> java.lang.Thread
> 1949:         // states will still report that the thread is blocked trying to
> 1950:         // acquire it.

Q: I have a concern here. Did we have a similar inconsistency before? As I see, 
this can be observable not only by thread dumps but also by JVMTI in general 
(independently of the thread's suspend status). Dan, can you comment on this, 
please?

test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait.java
 line 52:

> 50:  * @compile SuspendWithObjectMonitorWait.java
> 51:  * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait 
> SuspendWithObjectMonitorWait 3
> 52:  */

Q: I'm not that happy with adding this complexity into one single test. Would 
it make sense to split `doWork1`, `doWork2` and `doWork3` tests into 
independent test sharing some parts, e.g. .cpp file, 
`SuspendWithObjectMonitorWaitWorker` class etc.? Then the only duplication will 
be the `main()` method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2535945583
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2535957986

Reply via email to