RFR: 8364343: Virtual Thread transition management needs to be independent of JVM TI [v3]

[David Holmes]

On Thu, 20 Nov 2025 20:52:05 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> 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:
> 
>   Add Alan's comment in VirtualThread

src/hotspot/share/classfile/javaClasses.cpp line 1757:

> 1755:   jint* addr = java_thread->field_addr<jint>(_VTMS_transition_disable_count_offset);
> 1756:   int val = AtomicAccess::load(addr);
> 1757:   AtomicAccess::store(addr, val + 1);

Suggestion:

  AtomicAccess::inc(addr);

src/hotspot/share/classfile/javaClasses.cpp line 1764:

> 1762:   jint* addr = java_thread->field_addr<jint>(_VTMS_transition_disable_count_offset);
> 1763:   int val = AtomicAccess::load(addr);
> 1764:   AtomicAccess::store(addr, val - 1);

Suggestion:

  AtomicAccess::dec(addr);

src/hotspot/share/opto/runtime.hpp line 740:

> 738:     return vthread_transition_Type();
> 739:   }
> 740: 

I do not know C2 but this looks really strange - 4 different functions all return the same thing. ???

src/hotspot/share/runtime/handshake.cpp line 374:

> 372:     JavaThread* target = java_lang_Thread::thread(carrier_thread);
> 373:     assert(target != nullptr, "");
> 374:     // Technically there is need for a ThreadsListHandle since the target

Suggestion:

    // Technically there is no need for a ThreadsListHandle since the target

?

src/hotspot/share/runtime/mountUnmountDisabler.cpp line 147:

> 145:       MonitorLocker ml(VTMSTransition_lock);
> 146:       while (is_start_transition_disabled(current, vth())) {
> 147:         ml.wait(200);

I see a lot of timed-waits throughout this code. Is that because we poll rather than synchronizing properly? All this potential busy-waiting is surely going to cause performance glitches.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2547864726
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2547863852
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2547884313
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2547900707
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2547963241