Hi David,
On 11/29/18 8:05 PM, David Holmes wrote:
Hi Patricio,
This seems very complicated and I'm not quite seeing how it all goes
together. The check for waiting to re-lock now seems to dominant the
test and obscure the original checks.
I'm not sure this is worthwhile in the context of this test. It might
be much simpler to just get rid of the existing "waiting to re-lock"
check which will not be seen and then if we really want to check that
case add a much simpler form that just checks for that.
Ok, I actually had similar thoughts while I was adding the extra code.
Here is the new webrev:
http://cr.openjdk.java.net/~pchilanomate/8214148.02/webrev/
I removed the check for the "waiting to re-lock" case. I'm not sure if
it's okay to keep the change to
serviceability/tmtools/jstack/utils/DefaultFormat.java then. It doesn't
really affect this test, but it is needed for jstack to detect the locks
that appear in the stack report with the message "waiting to re-lock in
wait()".
Thanks,
Patricio
To me the simplest way to see the "re-lock in wait" case is to just:
synchronized(obj) {
obj.notifyAll();
<= take stack dump here =>
}
Cheers,
David
On 30/11/2018 5:52 am, Daniel D. Daugherty wrote:
Adding serviceability-dev@... since the 'tmtools' including 'jstack'
are owned by the Serviceability team.
Dan
On 11/28/18 4:21 PM, Patricio Chilano wrote:
Hi all,
Could you review this fix for test
serviceability/tmtools/jstack/WaitNotifyThreadTest.java?
On one hand the test was not properly checking what it was intended
to check, since as mentioned in JBS the logic for checking the
method name was wrong. Also since there was only one monitor in the
test, the "for" loop with the message "waiting to re-lock in wait()"
was never actually reached.
Additionally, with change 8150689 the message "waiting to re-lock in
wait()" is now shown in the frame where the relocking is actually
taking place, so the logic for checking that should change.
I fixed the first issues and added logic to check for the "waiting
to re-lock in wait()" case. I used the Thread.State attribute to
check desire states are reached before getting the thread dump
reports through jstack. I run the test in mach5 several times for
all platforms and they all passed.
Webrev URL: http://cr.openjdk.java.net/~pchilanomate/8214148.01/webrev
Bug URL: https://bugs.openjdk.java.net/browse/JDK-8214148
<https://bugs.openjdk.java.net/browse/JDK-8150689>
Thanks,
Patricio
