On Wed, 8 Jul 2026 22:08:07 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 JV...
>
> Patricio Chilano Mateo has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Dan's comments
> - David's comments
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/libStopThreadTest2.cpp
line 47:
> 45: JNIEXPORT void JNICALL
> 46: Java_StopThreadTest2_stopThread(JNIEnv* jni, jclass cls, jthread thread,
> jobject exception) {
> 47: jvmtiError err = jvmti->StopThread(thread, exception);
Nit: Unneeded double space after the `=` sign.
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/libStopThreadTest2.cpp
line 72:
> 70: err = jvmti->AddCapabilities(&caps);
> 71: if (err != JVMTI_ERROR_NONE) {
> 72: LOG("Agent_OnLoad: error in JVMTI AddCapabilities: %d\n", err);
This should return the `JNI_ERR` if the capabilities failed to be added.
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/libStopThreadTest2.cpp
line 75:
> 73: }
> 74:
> 75: printf("Agent_OnLoad: finished\n");
Better to use `LOG` instead of `printf` consistently.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3551104916
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3551089181
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3551098828
