On Thu, 9 Jul 2026 19:52:03 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:
>
> - Updates in libStopThreadTest2.cpp
> - More comments from David
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/StopThreadTest2.java
line 27:
> 25: * @test
> 26: * @bug 8386116
> 27: * @summary Test suspend and send async exception to a yielding virtual
> thread
Nits:
- s/Test suspend and send async/Test suspending and sending async/
- Also, it'd be better to add: `* @requires vm.jvmti` .
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/StopThreadTest2.java
line 50:
> 48: Thread.yield();
> 49: }
> 50: } catch (Throwable t) {}
Nit: Ignoring the exception does not look good. Would it better, at least, to
report the `Throwable` and related stack trace? Should the test fail in such
cases? We may want to filter our the expected exception (triggered by the JVMTI
`StopTread` and also verify if it was really thrown and fail otherwise.
I'm not sure, my suggestions are correct but wanted to check on this with you.
test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/libStopThreadTest2.cpp
line 51:
> 49: // so we ignore JVMTI_ERROR_OPAQUE_FRAME.
> 50: if (err != JVMTI_ERROR_OPAQUE_FRAME) {
> 51: check_jvmti_status(jni, err, "Error during StopThread()");
Since native method `stopThread()` ignores `JVMTI_ERROR_OPAQUE_FRAME`, the test
can pass even if every `StopThread` call returns OPAQUE_FRAME and no async
exception is ever delivered. But I'm not sure what can be done here to ensure
the async exception is delivered and also keep the test stable. Q: Would it
make sense to count the `OPAQUE_FRAME` errors, so sum of the number of threads
with async exceptions and with `OPAQUE_FRAME` is equal to total number of the
test threads?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3556311465
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3556348550
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3556380568
