On Fri, 16 Jan 2026 08:00:01 GMT, Serguei Spitsyn <[email protected]> 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
