RFR: 8366659: ObjectMonitor::wait() liveness problem with a suspension request [v31]

[Anton Artemov]

On Fri, 23 Jan 2026 21:42:46 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Anton Artemov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 56 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into JDK-8366659-OM-wait-suspend-deadlock
>>  - 8366659: Changed condition on when to post an event.
>>  - 8366659: Fixed whitespaces.
>>  - 8366659: Addressed reviewer's comments.
>>  - 8366659: Addressed reviewer's comments.
>>  - 8366659: Addressed reviewers' comments.
>>  - 8366659: Fixed whitespace.
>>  - 8366659: Addressed reviewer's comments.
>>  - 8366659: Addressed reviewers' comments, added comments, renamed tests.
>>  - 8366659: Modified the comment.
>>  - ... and 46 more: https://git.openjdk.org/jdk/compare/fa20391e...31779cad
>
> src/hotspot/share/runtime/objectMonitor.cpp line 1942:
> 
>> 1940:       }
>> 1941: 
>> 1942:       JvmtiExport::post_monitor_waited(current, this, ret == OS_TIMEOUT);
> 
> OK so this effectively polls and handles a pending suspend request.
> That does not guarantee that another suspend request won't be pending
> again (and handled in the `JvmtiExport::post_monitor_waited()` call).

I may not fully understand the problem here, but isn't it that suspension requests will be processed in order of execution at relevant places in the code? 

If there are, for instance 2 of those, then n1 will be handled on L1939, n2 - inside of `JvmtiExport::post_monitor_waited()`, any further suspension request will be handled by the thread as soon as it runs into a check for suspension in the program flow. What exactly is the concern here? There is no harm to suspend the thread in both L1939 and L1942 as the thread is known not to be the successor.

@sspitsyn could you clarify if it is technically possible to have more than one suspension request for a thread at any given time?

> src/hotspot/share/runtime/objectMonitor.cpp line 2099:
> 
>> 2097:     // move the add-to-entry_list operation, above, outside the critical section
>> 2098:     // protected by _wait_set_lock.  In practice that's not useful.  With the
>> 2099:     // exception of  wait() timeouts and interrupts the monitor owner
> 
> nit: extra space before wait.

Addressed.

> src/hotspot/share/runtime/objectMonitor.cpp line 2242:
> 
>> 2240:   // once we re-acquire the monitor we know if we need to throw IE or not.
>> 2241:   ObjectWaiter::TStates state = node->TState;
>> 2242:   assert(was_notified || state == ObjectWaiter::TS_RUN, "");
> 
> assert with multiple conditions should output the values so we know exactly what condition failed.

I added an error message and a small function to convert `ObjectWaiter::TState` to a `char*` for printing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2727865056
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2727865334
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2727865876