On Thu, 17 Jul 2025 10:12:16 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
> If JVMTI `StopThread` is done when the thread is in certain various states > (but not all states), after the `async` exception is delivered and handled, > hotspot leaves the thread's `interrupted` flag set. The end result is the > next time the thread does something like `Thread.sleep()`, it will > immediately get an `InterruptedException`. > > The fix is to clear the `interrupted` flag in the > `JavaThread::handle_async_exception()` after an `async` pending exception has > been set to be thrown with the `set_pending_exception()`. > > There are a couple of concerns with this fix which would be nice to sort out > with reviewers: > 1. The proposed fix may clear the interrupt state when it was already set > prior to the issuing of the `StopThread()` (this concern was raised by > @dholmes-ora in a comment of this JBS issue) > 2. The impacted code path is shared between the class > `InstallAsyncExceptionHandshakeClosure` used by the JVMTI `StopThread` > implementation and the class `ScopedAsyncExceptionHandshakeClosure` used by > the `ScopedMemoryAccess` > > I feel that clearing the `interrupted` flag byt the > `JavaThread::handle_async_exception()` is a right thing to do even though it > was set before the call to `JavaThread::install_async_exception()`. Also, it > has to be done for both `StopThread` and `ScopedMemoryAccess`. > > The fix also includes minor tweaks of the test `StopThreadTest` to make the > issue reproducible with it. > > Testing: > - Mach5 tiers 1-6 are passed > - Ran the updated reproducer test > `hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest` I also don’t see the purpose of setting the interrupted flag if all we want is for the target to wake up if it’s in one of the blocking calls. From the 3 cases in `JavaThread::interrupt()`, the only one where I see setting that flag is needed is for `Thread.sleep` because we check it to bail out, otherwise we would just keep sleeping for the remaining time. But maybe we could also add a `has_async_exception_condition()` check to bailout. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26365#issuecomment-3084708506
