RFR: 8373366: HandshakeState should disallow suspend ops for disabler threads [v13]

[Patricio Chilano Mateo]

On Fri, 16 Jan 2026 08:00:01 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> An asynchronous handshake operation (`ThreadSelfSuspensionHandshakeClosure`) can be installed when the target thread is not in a `MountUnmountDisabler` scope. But the target thread can enter such scope by the time the operation is self-processed by the target thread.
>> 
>> This is fixed by a small tweak in the function
>> `HandshakeOperation* HandshakeState::get_op_for_self(bool allow_suspend, bool check_async_exception)`.
>> The tweak is to skip a `HandshakeOperation` if `_handshakee->is_vthread_transition_disabler() == true`, so the same temporary suspension disabling mechanism would be used as for `_handshakee->is_disable_suspend() == true`.
>> 
>> All other changes are to move the `is_vthread_transition_disabler()` out of DEBUG to product.
>> 
>> Testing:
>>  - In progress: mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8375362: deadlock with unmount of suspended virtual thread interrupting another virtual thread

Thanks for the update Serguei, looks good to me. A few comments below.

src/hotspot/share/runtime/javaThread.cpp line 1186:

> 1184: bool JavaThread::java_suspend(bool register_vthread_SR) {
> 1185:   // Suspending a vthread transition disabler can cause deadlocks.
> 1186:   // The HandshakeState::has_operation does not allow such suspends.

So for a thread trying to self-suspend we don't use handshakes. We identify this case and call `do_owner_suspend()` directly, which would now hit the new assert added there. I think self-suspend within a `MountUnmountDisabler` scope is actually possible if some event is posted during the `interrupt` Java upcall. But maybe we should fix this case in a separate issue? (it's preexistent to this change)

src/hotspot/share/runtime/javaThread.hpp line 737:

> 735: private:
> 736:   bool _is_in_vthread_transition;                    // thread is in virtual thread mount state transition
> 737:   bool _is_vthread_transition_disabler;              // thread currently disabled vthread transitions

Should we make this field `JVMTI` only?

src/hotspot/share/runtime/suspendResumeManager.cpp line 136:

> 134:   while (is_suspended()) {
> 135:     log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended", p2i(_target));
> 136:     assert(!_target->is_vthread_transition_disabler(), "attempt to suspend a vthread transition disabler");

We could check this once before the loop since only the current thread changes `_is_vthread_transition_disabler`.

test/hotspot/jtreg/ProblemList.txt line 2:

> 1: #
> 2: # Copyright (c) 2016, 2026, Oracle and/or its affiliates. All rights reserved.

Not needed anymore.

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

PR Review: https://git.openjdk.org/jdk/pull/28740#pullrequestreview-3684247351
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2710046216
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2710038316
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2710050645
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2710059988