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

Coleen Phillimore coleenp at openjdk.org
Wed Nov 26 00:01:00 UTC 2025


On Tue, 25 Nov 2025 22:59:59 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:
> 
>   keep preexisting rebind order for mount

First read through, mostly questions and plea for comments.    This is a nice refactoring and cleanup of some very difficult code.  You don't have to do the renaming that I requested now if you don't want to.

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

> 1686: int java_lang_Thread::_jvmti_thread_state_offset;
> 1687: int java_lang_Thread::_VTMS_transition_disable_count_offset;
> 1688: int java_lang_Thread::_is_in_VTMS_transition_offset;

Since you're renaming these anyway, can we drop the VTMS part?  Just call it vthread_transition_disable_count_offset and is_in_vthread_transition_offset?  There are other VTMS named things that aren't these flags but they can stay.  Maybe migrate other names at some future point.

src/hotspot/share/opto/library_call.cpp line 3046:

> 3044: }
> 3045: 
> 3046: bool LibraryCallKit::inline_native_vthread_start_transition(address funcAddr, const char* funcName, bool is_final_transition) {

Would it be helpful to add a comment above this to say what this does? This is supposed to match some non-intrinsic code and might be helpful if you referenced that here.

src/hotspot/share/prims/jvm.cpp line 3671:

> 3669: 
> 3670: JVM_ENTRY(void, JVM_VirtualThreadStartFinalTransition(JNIEnv* env, jobject vthread))
> 3671:   oop vt = JNIHandles::resolve_external_guard(vthread);

Why do the opto runtime versions set is_in_VTMTS_transition in both the java.lang.Thread and JavaThread and these don't?

src/hotspot/share/prims/jvmtiEnv.cpp line 1827:

> 1825: JvmtiEnv::ClearAllFramePops(jthread thread) {
> 1826:   ResourceMark rm;
> 1827:   MountUnmountDisabler disabler(thread);

Not for this change but I thought JVMTI had some xml code that generated prefixes for these functions.  This seems like something that could be unified somewhere tbd.

src/hotspot/share/prims/jvmtiEnvBase.cpp line 1772:

> 1770: 
> 1771:   assert(java_thread != nullptr, "sanity check");
> 1772:   assert(!java_thread->is_in_VTMS_transition(), "sanity check");

Why don't you need these asserts anymore?

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

> 1150: bool JavaThread::is_in_VTMS_transition() const {
> 1151:   return AtomicAccess::load(&_is_in_VTMS_transition);
> 1152: }

Is the JavaThread version always the same as the java_lang_Thread::is_in_VTMS_transition(threadOop()) value?

src/hotspot/share/runtime/mountUnmountDisabler.hpp line 34:

> 32: 
> 33: class MountUnmountDisabler : public AnyObj {
> 34:   static volatile int _global_start_transition_disable_count;

Can you describe this variable - when is it set and why is there a global disabler? What does it mean to have 'n' active disablers?

A comment at the beginning of MountUnmountDisabler to say something of the effect that during virtual thread mounting and unmounting, JVMTI and operations that need to examine thread state need to be disabled.  Or is it the converse?  During JVMTI and operations that examine the state of threads, virtual thread mounting and unmounting must wait until these operations are complete.  This class is for the latter right?

src/hotspot/share/runtime/mutexLocker.cpp line 52:

> 50: Mutex*   JvmtiThreadState_lock        = nullptr;
> 51: Monitor* EscapeBarrier_lock           = nullptr;
> 52: Monitor* VTMSTransition_lock          = nullptr;

oh you could drop the name VTMS and call it VThreadTransitionLock can't you?

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

PR Review: https://git.openjdk.org/jdk/pull/28361#pullrequestreview-3507302896
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561864174
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561876549
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561897865
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561904709
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561910057
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561926510
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561943253
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2561945493


More information about the graal-dev mailing list