RFR 8214148: [TESTBUG] serviceability/tmtools/jstack/WaitNotifyThreadTest.java is not doing what is expected

[Patricio Chilano]

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 at ... 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
>>