On Mon, 18 Dec 2023 17:09:59 GMT, Serguei Spitsyn <[email protected]> wrote:
>> This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1
>> time frame.
>> It is fixing a deadlock issue between `VirtualThread` class critical
>> sections with the `interruptLock` (in methods: `unpark()`, `interrupt()`,
>> `getAndClearInterrupt()`, `threadState()`, `toString()`),
>> `JvmtiVTMSTransitionDisabler` and JVMTI `Suspend/Resume` mechanisms.
>> The deadlocking scenario is well described by Patricio in a bug report
>> comment.
>> In simple words, a virtual thread should not be suspended during
>> 'interruptLock' critical sections.
>>
>> The fix is to record that a virtual thread is in a critical section
>> (`JavaThread`'s `_in_critical_section` bit) by notifying the VM/JVMTI about
>> begin/end of critical section.
>> This bit is used in `HandshakeState::get_op_for_self()` to filter out any
>> `HandshakeOperation` if a target `JavaThread` is in a critical section.
>>
>> Some of new notifications with `notifyJvmtiSync()` method is on a
>> performance critical path. It is why this method has been intrincified.
>>
>> New test was developed by Patricio:
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock`
>> The test is very nice as it reliably in 100% reproduces the deadlock without
>> the fix.
>> The test is never failing with this fix.
>>
>> Testing:
>> - tested with newly added test:
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock`
>> - tested with mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 10 commits:
>
> - Merge
> - review: improve an assert message
> - review: moved a couple of comments out of try blocks
> - review: moved notifyJvmtiDisableSuspend(true) out of try-block
> - review: 1) replace CriticalLock with DisableSuspend; 2) minor tweaks
> - review: (1) rename notifyJvmti method; (2) add try-final statements to
> VirtualThread methods
> - Resolved merge conflict in VirtualThread.java
> - added @summary to new test SuspendWithInterruptLock.java
> - add new test SuspendWithInterruptLock.java
> - 8311218: fatal error: stuck in
> JvmtiVTMSTransitionDisabler::VTMS_transition_disable
src/hotspot/share/prims/jvm.cpp line 4024:
> 4022: #else
> 4023: fatal("Should only be called with JVMTI enabled");
> 4024: #endif
You can't do this! The Java code knows nothing about JVM TI being
enabled/disabled and will call this function unconditionally.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17011#discussion_r1432241016
