Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v7]

2023-12-18 Thread Serguei Spitsyn
On Fri, 15 Dec 2023 10:49:56 GMT, Serguei Spitsyn  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 incrementally with one 
> additional commit since the last revision:
> 
>   review: improve an assert message

Alan and Leonid, thank you for review!
Will push after the final mach5 testing is completed.

-

PR Comment: https://git.openjdk.org/jdk/pull/17011#issuecomment-1860980377


Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v7]

2023-12-17 Thread Leonid Mesnik
On Fri, 15 Dec 2023 10:49:56 GMT, Serguei Spitsyn  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 incrementally with one 
> additional commit since the last revision:
> 
>   review: improve an assert message

Marked as reviewed by lmesnik (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17011#pullrequestreview-1785514646


Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v7]

2023-12-15 Thread Serguei Spitsyn
> 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 incrementally with one additional 
commit since the last revision:

  review: improve an assert message

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17011/files
  - new: https://git.openjdk.org/jdk/pull/17011/files/917dc724..6f8cdf06

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17011=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=17011=05-06

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17011.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17011/head:pull/17011

PR: https://git.openjdk.org/jdk/pull/17011