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...
For other readers, Patricio has given me (and others) a solid walk-through of
the code changes and the refactoring so I am no longer concerned about
separating them out.
I still have some concerns about some of the details though. And a number of
minor nits.
Thanks
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2428:
> 2426:
> 2427: void
> 2428: StopThreadClosure::doit(JavaThread *target) {
Suggestion:
StopThreadClosure::doit(JavaThread* target) {
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2435:
> 2433:
> 2434: void
> 2435: StopThreadClosure::do_thread(Thread *target) {
Suggestion:
StopThreadClosure::do_thread(Thread* target) {
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2469:
> 2467: vframeStream vfst(_target_jt);
> 2468: Method* m = vfst.method();
> 2469: if (is_async_unsafe_method(m)) {
I assume `m` is the current Java method. Simply checking if that method is
unsafe seems inadequate as we may be in the VT code but calling a method of
another class when we process the async-exception. Doesn't this need to be an
actual walk of the stack to look for any unsafe method?
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2476:
> 2474: }
> 2475:
> 2476: if (_target_jt->at_no_async_entry_count() > 0
The count is set by the target thread but is read here by the current thread -
we need some use of atomics here.
src/hotspot/share/prims/jvmtiEnvBase.hpp line 545:
> 543: _exception(thread, exception) {}
> 544: void doit(JavaThread *target);
> 545: void do_thread(Thread *target);
Suggestion:
void doit(JavaThread* target);
void do_thread(Thread* target);
src/hotspot/share/runtime/javaThread.hpp line 236:
> 234: friend class HandshakeState;
> 235:
> 236: int _at_no_async_entry_count;
Suggestion:
int _at_no_async_entry_count; // Tracks whether we are within a
JRT_ENTRY_NOASYNC function.
src/hotspot/share/runtime/javaThread.hpp line 1355:
> 1353:
> 1354: class AtNoAsyncEntryMark : public StackObj {
> 1355: JavaThread *_target;
Suggestion:
JavaThread* _target;
src/hotspot/share/runtime/javaThread.hpp line 1356:
> 1354: class AtNoAsyncEntryMark : public StackObj {
> 1355: JavaThread *_target;
> 1356: bool _count;
Suggestion:
bool _no_async;
src/hotspot/share/runtime/javaThread.hpp line 1358:
> 1356: bool _count;
> 1357: public:
> 1358: AtNoAsyncEntryMark(JavaThread *t, bool b)
Suggestion:
AtNoAsyncEntryMark(JavaThread* t, bool allows_async)
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/StopThreadTest2.java
line 63:
> 61: suspendAllVirtualThreads();
> 62: for (Thread vthread : vthreads) {
> 63: stopThread(vthread, new ThreadDeath());
`ThreadDeath` is deprecated for removal so perhaps better to define a custom
exception type, or just use a generic type.
-------------
Changes requested by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/31759#pullrequestreview-4651046188
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541332382
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541342130
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541372665
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541395355
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541377408
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541386770
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541399581
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541411660
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541400291
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3541429023
