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

[David Holmes]

On 6/01/2015 10:56 PM, shanliang wrote:
> David Holmes wrote:
>> 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.
> You are right! I have changed back to Vector:
>     http://cr.openjdk.java.net/~sjiang/JDK-8068418/02/
>
> I forgot Vetor is synchronized, so long time not use it.

:) Thanks - seems okay to me.

David


> Thanks,
> Shanliang
>>
>> 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
>>>
>