RFR: 8281615: Deadlock caused by jdwp agent [v4]

[David Holmes]

On Wed, 16 Feb 2022 00:16:48 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> There are scenarios that JDWP agent can deadlock on `classTrackLock` monitor. Following is the scenario in bug report.
>> 
>> **Java Thread** 
>> -   loads a class and post `JVMTI_EVENT_CLASS_PREPARE` event
>> - JDWP event callback handler calls `classTrack_processUnloads()` to handle the event.
>> - `classTrack_processUnloads()` takes `classTrackLock` lock, then tries to allocate a new bag under the lock.
>> - bag allocation code calls` jvmtiAllocate()`, which may be blocked by ongoing safepoint due to state transition.
>> 
>> If the safepoint is GC safepoint (prior to JDK16) or `VM_JvmtiPostObjectFree`  safepoint (JDK16 or later)
>> 
>> **VM Thread**
>> - post `JVMTI_EVENT_OBJECT_FREE`
>> - JDWP event callback handler calls `cbTrackingObjectFree()` to handle the event
>> - `cbTrackingObjectFree()` tries to acquire `classTrackLock` lock, leads to deadlock
>> 
>> From my research, there are three events that may be posted at safepoints, `JVMTI_EVENT_GARBAGE_COLLECTION_START`, `JVMTI_EVENT_GARBAGE_COLLECTION_FINISH` and  `JVMTI_EVENT_OBJECT_FREE`, but only  `JVMTI_EVENT_OBJECT_FREE` is relevant to JDWP agent.
>> 
>> The solution I purpose here, is simply move allocation/deallocation code outside of `classTrackLock` lock.
>> 
>> 
>> Test:
>> - [x] tier1 
>> - [x] vmTestbase_nsk_jdi
>> - [x] vmTestbase_nsk_jdwp
>> - [x] vmTestbase_nsk_jvmti
>
> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Simplify classTrack_reset and revert bagSize check

The correctness here depends on exactly how these different functions can be called. I would have expected the allowed sequence to be:
activate -> 1or more processUnloads -> reset

but given processUnloads can somehow occur concurrently with activate, there is obviously more at play here. The main question for me is whether reset can be called more than once and whether it too can be called when processUnloads is still executing? If yes then there are still problems.

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

PR: https://git.openjdk.java.net/jdk/pull/7461