On Mon, 17 Nov 2025 20:19:58 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 would be to
> place extra overhead on the thread requesting to disable transitions (e.g. by
> usi
ng a safepoint, handshake-all, or UseSystemMemoryBarrier). Performance
analysis show no difference with current mainline so I believe this approach is
simpler.
>
> - 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 terms of mount and unmount cases, and
> a...
src/java.base/share/classes/java/lang/VirtualThread.java line 1390:
> 1388: }
> 1389:
> 1390: // -- JVM TI support --
We'll need to update is comment as it no longer only for JVMTI.
This might be a good place for a block comment to define "transitions" covering
the changing of thread identity the continuation mount/unmount, and how the
notification to the VM support JVMTI and handshakes. Maybe I could contribute
a block comment to include here?
src/java.base/share/native/libjava/VirtualThread.c line 38:
> 36: { "startFinalTransition", "()V", (void *)&JVM_VirtualThreadEnd },
> 37: { "startTransition", "(Z)V", (void
> *)&JVM_VirtualThreadStartTransition },
> 38: { "endTransition", "(Z)V", (void
> *)&JVM_VirtualThreadEndTransition },
I wonder if JVM_VirtualThreadStart and JVM_VirtualThreadEnd should be renamed
to have EndFirstTransition and StartFinalTransaction in the names so it's easy
to follow through from the Java code down to
MountUnmountDisabler::start_transition/end_transition.
test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreadsWhenParking.java
line 94:
> 92: });
> 93: }
> 94: // wait for all virtual threads to start so all have a
> non-empty stack
This reminds me the loom repo has a small update to to the
DumpThreadsWithEliminatedLock.java test to ensure that the virtual thread
starts execution before doing the thread dump. This was noticed with
test-repeat runs of the new test to ensure it was stable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2542097138
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2542016761
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2542034248
