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

Changes seem reasonable given the discussion.

Thanks,
David

I thought from:
https://github.com/openjdk/jdk/pull/7461#issuecomment-1043215122
there was srtill an outstanding issue with the handlerLock ? Though @plummercj  seems to have approved anyway.

src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c line 91:

> 89:      *
> 90:      * Note: jvmtiAllocate/jvmtiDeallocate() may be blocked by ongoing safepoints.
> 91:      * It is dangerous to call them (via bagCreateBag/bagDestroyBag())while holding monitor(s),

Nit: need space after ))

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

Marked as reviewed by dholmes (Reviewer).

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