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

[Daniel D. Daugherty]

On 10/3/19 11:58 PM, 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/

src/hotspot/share/services/threadService.cpp
     L399:     // waiting on both a raw monitor and ObjectMonitor at the 
same time. It
         s/waiting on/waiting to lock/

     L981:      const char* owner_desc = ",\n  which is held by";
     L994:            st->print_cr(",\n  which has now been released");
     L1008:         owner_desc = "\n  in JNI, which is held by";
         Not your bug for L981 or L1008, but those '\n' aren't portable 
right?
         I think a 'st->cr()' is needed instead.
         Of course, I don't understand why the newline is there anyway,
         but without seeing an example output its hard to say.

Thumbs up. Don't need to see another webrev...

Dan


>
> The only change is threadService.cpp
>
> Thanks,
> David