jmx-dev RFR 8067241 DeadlockTest.java failed with negative timeout value

[shanliang]

Jaroslav Bachorik wrote:
> Hi,
>
> On 12/12/2014 09:56 AM, shanliang wrote:
>> Daniel Fuchs wrote:
>>> Hi Shanliang,
>>>
>>> These two statements are no longer needed and should be removed - as 
>>> they
>>> are misleading:
>>>
>>>    64         if (!bb.gotLock) {
>>>    65             throw new RuntimeException("Failed to get lock, 
>>> impossible!");
>>>    66         }
>>>
>>>
>>>    81         if (!wb.done) {
>>>    82             throw new RuntimeException("It is blocked!");
>>>    83         }
>>>
>> Yes, can be removed.
>>>
>>>
>>> which makes me wonder whether your changes are deeper than intended.
>>> Now the test will either succeed, or fail in jtreg timeout. The 
>>> condition
>>> "It is blocked!"
>>> can no longer be detected by the test. Is that your intention? If 
>>> that's
>>> your intention - I guess it is OK - but maybe it would deserve a 
>>> comment
>>> in the @summary of the test. Something like 'This bug attempts to
>>> reproduce
>>> a deadlock situation and will block forever if the deadlock is 
>>> present.'
>> Updated.
>>
>> Here is the new version:
>> http://cr.openjdk.java.net/~sjiang/JDK-8067241/01/
>
> I know that I'm starting to repeat myself but couldn't you replace 
> L58-61 and L91-95 with a simple Phaser?
>
> Like
> ```
>
> L58-61: Phaser p = new Phaser(2); p.arriveAndAwaitAdvance()
>
> Then pass the Phaser instance to BadBoy and
>
> L91-95: p.arriveAndAwaitAdvance();
> ```
>
> This way the main thread is blocked till the BadBoy thread has 
> acquired the BadBoy's lock and the code is more concise.
I agree Daniel's comment that we may need more investigation for using 
Phaser, Phaser might be a good solution but the current synchronization 
solution should work too and I have ran the test on jprt.

So let's keep the current solution and investivating on Phaser if the 
test fails again.

Shanliang
>
> -JB-
>
>
>>
>> Thanks,
>> Shanliang
>>
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 12/12/14 8:33 AM, shanliang wrote:
>>>> Hi,
>>>>
>>>> It is a test bug, it is not correct:
>>>>   while(!wb.done || timeToWait > 0) {
>>>>
>>>> it should be:
>>>>   while(!wb.done && timeToWait > 0) {
>>>>
>>>> || should be changed to &&
>>>>
>>>> Another issue is that the waiting time could be not enough (final
>>>> long timeout = 2000).
>>>>
>>>> The fix is to remove the waiting time specified in the test, the
>>>> timeout of test harness will be used as the max waiting time.
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8067241
>>>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-8067241/00/
>>>>
>>>> thanks,
>>>> Shanliang
>>>
>>
>