On Fri, 3 Jul 2026 02:18:32 GMT, David Holmes <[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...
>
> 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.
Yes, we can add a `DowncallLinker::downcall_stub_name()` method to encode it. I
didn’t want to generate additional churn touching platform dependent files.
> 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?
One is the method executed for platform threads, the other for virtual threads.
This comes from `AdapterClosure` used in `JvmtiHandshake::execute`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517380063
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3517380837
