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

[Jaroslav Bachorik]

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.

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