On Mon, 1 Dec 2025 23:00:16 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: > > Fix comment in inline_native_vthread_start_transition src/hotspot/share/runtime/mountUnmountDisabler.cpp line 88: > 86: // We rebinded on start_transition but the mount/unmount operation > 87: // failed so now we need to rebind to the original state. > 88: _current->rebind_to_jvmti_thread_state_of(_is_mount ? _vthread() > : _current->threadObj()); Q: Not sure I fully understand this comment. We have done a rebind at the line 67 for an unmount transition only. But this comment tells that it was done for mount transition as well. Also, before this update, we used to do a rebind of `JvmtiThreadState` to a vthread in the `JvmtiVTMSTransitionDisabler::VTMS_mount_end()` for the normal (not failed) case. Please, could you explain a little bit more? It feels like this comment needs a correction. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2580118016
