RFR 9: 8168517 : java/lang/ProcessBuilder/Basic.java failed

[Roger Riggs]

Hi David,

On 10/27/2016 4:51 PM, David Holmes wrote:
> On 28/10/2016 3:12 AM, Roger Riggs wrote:
>> Hi David,
>>
>> On 10/27/2016 12:57 PM, David Holmes wrote:
>>> Hi Roger,
>>>
>>> On 28/10/2016 1:44 AM, Roger Riggs wrote:
>>>> Please review a test fix for a timeout on a busy system in
>>>> Process.waitFor a destroyed process.
>>>
>>> Won't that now cause the test to hang until timed-out by the harness?
>> yes, but an in-app timeout is not much different than the harness 
>> timeout
>> so I didn't complicate the test. Under normal system loading it should
>> be 1-4 seconds.
>
> Okay, but then the logic following this change is redundant:
>
> 2407             int exitValue = p.waitFor();  // wait for it to be done
> 2408
> 2409             start = System.nanoTime();
> 2410             p.waitFor(8, TimeUnit.SECONDS);
> 2411             end = System.nanoTime();
> 2412
> 2413             if ((end - start) > TimeUnit.SECONDS.toNanos(7))
> 2414                 fail("Test failed: waitFor took too long on a 
> dead process. (" + (end - start) + "ns)"
> 2415                 + ", exitValue: " + exitValue);
Not entirely, it is intended to check that when waitFor is used with a 
destroyed process
it completes (pretty much) immediately.

Roger

>
> David
>
>> I contemplated increasing the timeout but given the issue is system 
>> loading
>> I didn't have a good idea what to raise it to.  30sec, 1 min, 2 min.
>> and the harness timeout is 2min.
>>
>> Roger
>>
>>>
>>> David
>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~rriggs/webrev-basic-destroy-8168517/
>>>>
>>>> Thanks, Roger
>>>>
>>