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

Reply via email to