RFR 6720349: NotificationBufferDeadlockTest.java throw exception: java.lang.Exception: TEST FAILED: Deadlock detected

[shanliang]

David,

Here is the new version, I have added the comments as you suggested, and 
I replaced the variable "sources" with a synchronized list.
    http://cr.openjdk.java.net/~sjiang/JDK-8068418/01/

Thanks for the review.
Shanliang

David Holmes wrote:
> Hi Shanliang,
>
> On 6/01/2015 3:47 AM, shanliang wrote:
>> Hi,
>>
>> Please review this fix:
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8068418
>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-8068418/00/
>>
>> This must be a timing issue in the test, the test called:
>>     t.join(5000L); to wait a thread's dying,  I reproduced the bug by
>> insert at line 202:
>>     try {
>>         Thread.sleep(5100);
>>     } catch (Exception ee) {}
>> to delay the t's dying.
>>
>> The fix is to replace:
>>     t.join(5000L);
>> by:
>>     t.join();
>>
>> and replace:
>>     Object.wait(timeout);
>> by
>>     CountDownLatch.countDown();
>
> Okay - you could have just done wait() but the switch to 
> CountDownLatch is somewhat cleaner.
>
>> The test harness timeout will be used as the max waiting timeout.
>
> So the test will now never report that it thinks it has hit a 
> deadlock, it will just timeout. But you added some extra printout so 
> okay I guess - but I suggest adding a comment at the top (where @run 
> etc are) saying that if test times out then deadlock is suspected.
>
> Minor nit:
>
>  System.out.println("DeadlockTest-addNotificationListener waiting the 
> XXX thread to die.
>
> should say "waiting for the XXX thread ..."
>
> Thanks,
> David
>
>> Shanliang