On Fri, 3 Jul 2026 00:49:43 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
> An asynchronous exception sent by JVMTI `StopThread` to a virtual thread can
> be processed and thrown at a point where it is unsafe to do so, leaving the
> virtual thread in an invalid state. The unsafe nature of `StopThread` is not
> an issue exclusive to virtual threads, but due to extra code executed during
> mount/unmount transitions, some tests that are safe for platform threads are
> not for virtual threads. In the reported bug, where the async exception is
> sent to a thread executing `Thread.yield` in a loop, the exception ends up
> being thrown on return from `VirtualThread.startTransition`. Since the
> transition bits remain set, the virtual thread hits the reported assert in
> `MountUnmountDisabler::start_transition` at the next unmount attempt.
>
> A similar issue can happen if the exception is thrown right after executing
> `VirtualThread.endFirstTransition` or right before
> `VirtualThread.startFinalTransition` which can be reproduced by calling
> `StopThread` on virtual threads with empty tasks.
>
> The patch fixes these cases and potentially others that can happen if an
> async exception is thrown while executing a method in the `VirtualThread`
> class, i.e. it improves the robustness of the implementation in the presence
> of async exceptions. It is not an attempt to make `StopThread` bulletproof as
> that would be a more ambitious task.
>
> The proposed changes add two extra checks before installing the asynchronous
> exception handshake. The first verifies that the top method is not a
> `VirtualThread` method. The second verifies that the exception will be thrown
> at the current bytecode, i.e. that exception processing will not be deferred
> to a later safepoint poll where the target might already be in one of the
> unsafe methods.
>
> A less restrictive alternative that avoids that second check is to have the
> target defer processing of the handshake as long as it’s unsafe to do so (as
> based on the first check above). If the handshake is still pending when an
> unmount transition begins, we process it and save the exception in the
> virtual thread’s `JvmtiThreadState` to be thrown at the end of the next
> mount. I have a patch implementing this approach, but the code is a bit more
> involved and I wasn’t convinced it was worth it.
>
> I also refactored `StopThread` by introducing `StopThreadClosure` and
> `StopThreadAsyncClosure` handshake classes. This aligns it with the other
> JVMTI methods that use `JvmtiHandshake`, and also keeps the `StopThread`
> specific logic local to the JVMTI code.
>
> The changes...
Sorry Patricio but I found the refactoring completely obscured the fix for me.
There are too many closures and apparent handshakes that make the control flow
very difficult to see, and I can't see where the code in `StopThread` went to -
e.g. `get_threadOop_and_JavaThread` - is it hidden on one of the existing
handshake classes? Can we split this out to do the refactor first, then apply
the unsafe checks?
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2405:
> 2403: return m->method_holder() == vmClasses::VirtualThread_klass()
> 2404: // Checks for VirtualThread$VThreadContinuation$1.run
> 2405: || (m->name() == vmSymbols::run_method_name() &&
> m->jvmti_hide_events());
Suggestion:
return (m != nullptr) &&
(m->method_holder() == vmClasses::VirtualThread_klass() ||
// Checks for VirtualThread$VThreadContinuation$1.run
(m->name() == vmSymbols::run_method_name() && m->jvmti_hide_events()));
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2409:
> 2407:
> 2408: static bool is_downcall_stub(CodeBlob* cb) {
> 2409: return cb != nullptr && cb->is_runtime_stub() && (strcmp(cb->name(),
> "nep_invoker_blob") == 0);
It is unfortunate to hardcode the name of a stub that is defined/used a long
way from here.
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2450:
> 2448: void
> 2449: StopThreadClosure::do_vthread(Handle target_h) {
> 2450: if (!_self && !JvmtiEnvBase::is_vthread_suspended(target_h(),
> _target_jt)) {
Why is a suspend check in two places? It is not at all clear to me how to the
two places relate in terms of the execution call chain. Is one done by the
handshaker and the other by the handshakee?
src/hotspot/share/runtime/javaThread.hpp line 1354:
> 1352: };
> 1353:
> 1354: class AtNoAsyncEntryMark : public StackObj {
What does "At" mean here? Please document what this class is doing.
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/libStopThreadTest2.cpp
line 46:
> 44:
> 45: JNIEXPORT void JNICALL
> 46: Java_StopThreadTest2_stopThread(JNIEnv *jni, jclass cls, jthread thread,
> jobject exception) {
Suggestion:
Java_StopThreadTest2_stopThread(JNIEnv* jni, jclass cls, jthread thread,
jobject exception) {
-------------
PR Review: https://git.openjdk.org/jdk/pull/31759#pullrequestreview-4622286361
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517068725
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517076922
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517083742
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517088050
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517089360
