> 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...
Patricio Chilano Mateo has updated the pull request incrementally with two additional commits since the last revision: - Dan's comments - David's comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/31759/files - new: https://git.openjdk.org/jdk/pull/31759/files/aa7e9184..2e6bfb0a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=31759&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=31759&range=00-01 Stats: 34 lines in 8 files changed: 6 ins; 0 del; 28 mod Patch: https://git.openjdk.org/jdk/pull/31759.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/31759/head:pull/31759 PR: https://git.openjdk.org/jdk/pull/31759
