RFR: 8325187: JVMTI GetThreadState says virtual thread is JVMTI_THREAD_STATE_INTERRUPTED when it no longer is [v12]

[David Holmes]

On Sat, 16 Mar 2024 22:33:49 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Please, review this fix correcting the JVMTI  `RawMonitorWait()` implementation.
>> The `RawMonitorWait()` is using the the  `jt->is_interrupted(true)` to update the interrupt status of the interrupted waiting thread.  The issue is that when it calls `jt->is_interrupted(true)` to fetch and clear the `interrupt status` of the virtual thread, this information is not propagated to the `java.lang.Thread` instance.
>> In the `VirtualThread` class implementation the `interrupt status` for virtual thread is stored for both virtual and carrier threads. It is because the implementation of object monitors for virtual threads is based on pinning virtual threads, and so, always operates on carrier thread. The fix is to clear the interrupt status for both virtual and carrier  `java.lang.Thread` instances.
>> 
>> Testing:
>>  - tested with new test `hotspot/jtreg/serviceability/jvmti/vthread/InterruptRawMonitor` which is passed with the fix and failed without it
>>  - ran mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: made current changes limitedto just RawMonitorWait

Thanks for doing this update.

> One minor concern is the `JavaThread::sleep_nanos(jlong nanos)` ?
> It is used by the `JavaThread::sleep(jlong millis)` which in tern is not used much.

There should be no virtual threads involved in executing the internal `sleep(millis)` variant. But good catch!

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

> 593: 
> 594: // Checks and clears the interrupt status for platform or virtual thread.
> 595: // Used by the JVMTI RawMonitorWait only.

Or more strongly:

// This is only for use by JVMTI RawMonitorWait. It emulates the actions of the Java code in Object::wait
// which are not present in RawMonitorWait.

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

PR Review: https://git.openjdk.org/jdk/pull/18093#pullrequestreview-1941874826
PR Review Comment: https://git.openjdk.org/jdk/pull/18093#discussion_r1527695544