Review request for 8029378: com/sun/jdi/BadHandshakeTest.java failed with java.util.concurrent.TimeoutException

[David Holmes]

On 16/01/2014 9:01 PM, shanliang wrote:
> Jaroslav Bachorik wrote:
>> On 16.1.2014 11:48, shanliang wrote:
>>> David Holmes wrote:
>>>> On 16/01/2014 5:37 PM, shanliang wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this simple fix, the test needs  more time to wait
>>>>> Phaser.awaitAdvanceInterruptibly(...).
>>>>
>>>> Integer.MAX_VALUE? There's no point using a timed form at all.
>>>>
>>>> David
>>> Yes Phaser.awaitAdvanceInterruptibly(int phase) could be used here, but
>>> the call of this method is wrapped in:
>>>     jdk.testlibrary.ProcessTools.startProcess(...)
>>>
>>> So we have to add a new method ProcessTools.startProcess(...) which has
>>> no timeout parameter. I did not do this because I thought to have a
>>> simple fix only within the test.
>>
>> This timeoud seems to be caused by using the fastdebug build. You
>> could use Utils.TIMEOUT_FACTOR to scale the original timeout (1500)
>> according to the parameters specified for tests running fastdebug builds.
> Yes it could be a solution, but to wait the jtreg timeout seems better,
> we do not need to take care of timeout.

Okay. It would have been clearer if you had stated that you were 
removing the timeout in the test and relying on jtreg timing out the 
test instead. :)

Thanks,
David

> Thanks,
> Shanliang
>>
>> -JB-
>>
>>>
>>> If this is useful, here is the new web:
>>> http://cr.openjdk.java.net/~sjiang/JDK-8029378/01/
>>>
>>> Thanks,
>>> Shanliang
>>>
>>>>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-8029378/00/
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8029378
>>>>>
>>>>> Thanks,
>>>>> Shanliang
>>>
>>
>