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

[shanliang]

David Holmes wrote:
> 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 and Jaroslav for reviewing, I will push the first version.

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