On Wed, 26 Nov 2025 22:44:17 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> When `ThreadSnapshotFactory::get_thread_snapshot()` captures a snapshot of a
>> virtual thread, it uses `JvmtiVTMSTransitionDisabler` class to disable
>> mount/unmount transitions. However, this only works if a JVMTI agent has
>> attached to the VM, otherwise virtual threads don’t honor the disable
>> request. Since this snapshot mechanism is used by `jcmd Thread.dump_to_file`
>> and `HotSpotDiagnosticMXBean` which don’t require a JVMTI agent to be
>> present, getting the snapshot of a virtual thread in such cases can lead to
>> crashes.
>>
>> This patch moves the transition-disabling mechanism out of JVMTI and into a
>> new class, `MountUnmountDisabler`. The code has been updated so that
>> transitions can be disabled independently of JVMTI, making JVMTI just one
>> user of the API rather than the owner of the mechanism. Here is a summary of
>> the key changes:
>>
>> - Currently when a virtual thread starts a mount/unmount transition we only
>> need to check if `_VTMS_notify_jvmti_events` is set to decide if we need to
>> go to the slow path. With these changes, JVMTI is now only one user of the
>> API, so we instead check the actual transition disabling counters, i.e the
>> per-vthread counter and the global counter. Since these can be set at any
>> time (unlike `_VTMS_notify_jvmti_events` which is only set at startup or
>> during a safepoint in case of late binding agents), we follow the classic
>> Dekker pattern for the required synchronization. That is, the virtual thread
>> sets the “in transition” bits for the carrier and vthread *before* reading
>> the “transition disabled” counters. The thread requesting to disable
>> transitions increments the “transition disabled” counter *before* reading
>> the “in transition” bits.
>> An alternative that avoids the extra fence in `startTransition` would be to
>> place extra overhead on the thread requesting to disable transitions (e.g.
>> using safepoint, handshake-all, or UseSystemMemoryBarrier). Performance
>> analysis show no difference with current mainline so for now I kept this
>> simpler version.
>>
>> - Ending the transition doesn’t need to check if transitions are disabled
>> (equivalent to not need to poll when transitioning from unsafe to safe
>> safepoint state). But we still require to go through the slow path if there
>> is a JVMTI agent present, since we need to check for event posting and JVMTI
>> state rebinding. As such, the end transition follows the same pattern that
>> we have today of only needing to check `_VTMS_notify_jvmti_events`.
>>
>> - The code was previously structured in t...
>
> Patricio Chilano Mateo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> missing to initialize _is_disabler_at_start
Excellent work. Thank you for answering my questions. There's one click
suggested change.
src/hotspot/share/opto/library_call.cpp line 3052:
> 3050: //
> 3051: // java_lang_Thread::set_is_in_VTMS_transition(vthread, true);
> 3052: // carrier->set_is_in_VTMS_transition(true);
Suggestion:
// java_lang_Thread::set_is_in_vthread_transition(vthread, true);
// carrier->set_is_in_vthread_transition(true);
src/hotspot/share/opto/library_call.cpp line 3058:
> 3056: // if (disable_requests > 0) {
> 3057: // slow path: runtime call
> 3058: // }
The comment is helpful, thanks.
-------------
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/28361#pullrequestreview-3526451726
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2578205190
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2578206636
