RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

[David Holmes]

Hi Serguei,

On 7/10/2019 6:42 pm, serguei.spitsyn at oracle.com wrote:
> Hi David,
> 
> Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI
> MonitorContendedEntered callback on an object monitor hold by thread T2.
> 
> In theory, if only raw monitor is used in further deadlock detection 
> analysis
> then a possible dependency loop with T1 an T2 involved won't be detected.
> For full analysis the dependency chain through object monitor have to be 
> also analyzed.
> This would add extra complexity into the deadlock detection code for 
> very rare corner case.
> 
> I do not care much about this, and so, like the approach.

To be clear the case you describe is not handled by either the current 
code nor my proposed update.

> The fix itself looks good to me.

Thanks.

David

> Thanks,
> Serguei
> 
> 
> On 10/3/19 20:58, David Holmes wrote:
>> Okay, to allow for me to make forward progress here I've decided to 
>> follow the "principle of least brokenness" ;-)
>>
>> Recap: Because of JVMTI event callbacks it is possible for a thread to 
>> set its current pending monitor as a JvmtiRawMonitor when it was 
>> already set for an ObjectMonitor. This is broken in at least two ways:
>> - when the raw monitor use completes the pending monitor is set to 
>> NULL, not restored to the ObjectMonitor
>> - whilst the raw monitor is seen as the pending monitor the 
>> ObjectMonitor is not considered by the deadlock detection logic
>>
>> Ignoring what I'm currently doing with jvmtiRawMonitor, I do not know 
>> how to fix the above brokenness and it is out of scope for what I am 
>> trying to do.
>>
>> So what I propose is to make things no more broken than they are now, 
>> and actually improve things a little:
>> - the pending JvmtiRawMonitor is given preference over the 
>> ObjectMonitor in the deadlock detection code (this emulates current 
>> situation of the raw monitor overwriting the pending ObjectMonitor)
>> - we no longer lose the fact we were also pending on an ObjectMonitor
>> - the stack printing code in the deadlock detector prints information 
>> about both the raw monitor and the ObjectMonitor
>>
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~dholmes/8231289/webrev.v2/
>>
>> The only change is threadService.cpp
>>
>> Thanks,
>> David
>