Hi Dan,
Thanks for the second look.
On 5/10/2019 1:51 am, Daniel D. Daugherty wrote:
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/
Fixed.
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.
\n within a string literal should be perfectly portable. It's a symbolic
representation of a "newline". If it eventually get written to somewhere
that cares (like a file) it will be converted into appropriate CR or
CRLF characters for the platform.
Of course, I don't understand why the newline is there anyway,
but without seeing an example output its hard to say.
Yes I assume it looks pretty if you see it written out.
Thumbs up. Don't need to see another webrev...
Thanks,
David
-----
Dan
The only change is threadService.cpp
Thanks,
David
