On Sat, 16 Mar 2024 22:33:49 GMT, Serguei Spitsyn <sspit...@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
