On Mon, 28 Jul 2025 06:57:42 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` > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > review: implemented a suggestion: do not set interrupt flag at all I've implemented and pushed the suggestion from Patricio. The mach5 tiers 1-6 are clean. I'm not sure about correctness of the tweak in the `JavaThread::sleep_nanos()`: @@ -2122,6 +2117,9 @@ bool JavaThread::sleep_nanos(jlong nanos) { jlong nanos_remaining = nanos; for (;;) { + if (has_async_exception_condition()) { + return false; + } // interruption has precedence over timing out if (this->is_interrupted(true)) { return false; The mach5 tiers 1-6 tests are all passed without this tweak. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26365#issuecomment-3125809023