On Wed, 10 Dec 2025 10:18:11 GMT, Serguei Spitsyn <[email protected]> wrote:
> This update fixes the following problem. The function
> JavaThread::java_suspend has the following assert:
>
> bool JavaThread::java_suspend(bool register_vthread_SR) {
> // Suspending a vthread transition disabler can cause deadlocks.
> assert(!is_vthread_transition_disabler(), "no suspend allowed for vthread
> transition disablers");
> . . .
>
> This is an over-hard requirement/invariant for threads that execute JVMTI
> functions (e.g. in an agent event handler). Such threads need a chance to be
> eventually suspended after JVMTI function completes its work and its
> `mountUnmountDisabler` is destructed.
>
> A more adequate approach is to tweak the function
> `HandshakeOperation* HandshakeState::get_op_for_self(bool allow_suspend, bool
> check_async_exception)`
> so it could 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`.
>
> Testing:
> - In progress: mach5 tiers 1-6
Hi Serguei, some comments below, thanks.
src/hotspot/share/runtime/handshake.cpp line 524:
> 522: assert(allow_suspend || !check_async_exception, "invalid case");
> 523: #if INCLUDE_JVMTI
> 524: if (allow_suspend && (_handshakee->is_disable_suspend() ||
> _handshakee->is_vthread_transition_disabler())) {
I see we need this because a thread could be suspended outside a disable
operation (e.g. waiting in `disable_transition_for_one()`) but only process the
suspend once inside it. Looking at the places where we use
`MountUnmountDisabler`, the one where I can clearly see this could happen is
`JvmtiEnv::InterruptThread` because of the upcall to Java. Did you encounter
any other places where we could process suspend requests inside a disabling
operation?
src/hotspot/share/runtime/javaThread.cpp line 1182:
> 1180: bool JavaThread::java_suspend(bool register_vthread_SR) {
> 1181: // Suspending a vthread transition disabler can cause deadlocks.
> 1182: assert(!is_vthread_transition_disabler(), "no suspend allowed for
> vthread transition disablers");
So except for self-suspend, how can we hit this assert if we use an exclusive
disabler? And if we remove it and allow a disabler to self-suspend, wouldn't we
deadlock since the resumer would be unable to run?
-------------
PR Review: https://git.openjdk.org/jdk/pull/28740#pullrequestreview-3569493854
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2612230146
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2612215094
