Hi Roger,
On 28/10/2016 10:46 AM, Roger Riggs wrote:
Hi David,
On 10/27/16 7:22 PM, David Holmes wrote:
On 28/10/2016 6:58 AM, Roger Riggs wrote:
Hi David,
On 10/27/2016 4:51 PM, David Holmes wrote:
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.
But this is the second waitFor call after the process is destroyed.
Sorry I don't really see the point.
The tests were added to determine if waitFor(timeout) was handling the
timeout parameter correctly.
The 2nd test here was to check the datapath when the process already
been destroyed.
The 'extra' waitFor() ensures the process is gone.
But you are correct, this exercises a path through the waitFor code that
does not even look
at the timeout value so I'll remove the entire 2nd case.
Webrev updated in place.
http://cr.openjdk.java.net/~rriggs/webrev-basic-destroy-8168517/
Okay, but now you don't need the p.waitFor() after the destroy() at all.
Hmmm ... I guess either we want the original code with a longer timeout
to accommodate slow/loaded platforms (to test that waitFor(n) after a
destroy() is timely); or everything after destroy() can be deleted.
Cheers,
David
Thanks, Roger
David
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
