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
