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

[David Holmes]

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.

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
>