RFR: 8306324: StopThread results in thread being marked as interrupted, leading to unexpected InterruptedException [v5]

[Patricio Chilano Mateo]

On Wed, 30 Jul 2025 07:19:34 GMT, Serguei Spitsyn <sspitsyn at 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: restored the tweak in the JavaThread::sleep_nanos

Changes look good to me, just a few comments below.

src/hotspot/share/runtime/javaThread.cpp line 2121:

> 2119:   for (;;) {
> 2120:     if (has_async_exception_condition()) {
> 2121:       return false;

The comment above `JavaThread::sleep_nanos` should be updated about this case.
Also, back in `JVM_SleepNanos` we should probably skip throwing IE for this case (even though it will be overwritten by the async one later).

-------------

PR Review: https://git.openjdk.org/jdk/pull/26365#pullrequestreview-3072155539
PR Review Comment: https://git.openjdk.org/jdk/pull/26365#discussion_r2243072058