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

[David Holmes]

On 6/01/2015 9:03 PM, shanliang wrote:
> David,
>
> Here is the new version, I have added the comments as you suggested, and
> I replaced the variable "sources" with a synchronized list.

Why did you do that ?? Vector is synchronized so it was fine for the job.

David

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