Thank you Pavel!
On 3/14/19 2:02 PM, Pavel Rappo wrote:
I have to look at this patch in more detail, however here's what jumped out at
me straight away:
long deadline = System.nanoTime() + remainingNanos;
It seems like a possibility for an overflow.
Not quite. The deadline can surely become negative, which is alright.
Later we only check the difference (deadline - System.nanoTime()).
Actually, the new code mimics what we have in PocessImpl for Unix and
Windows.
With kind regards,
Ivan
Documentation for System.nanoTime
has a special section on this:
For example, to measure how long some code takes to execute:
long startTime = System.nanoTime();
// ... the code being measured ...
long elapsedNanos = System.nanoTime() - startTime;
To compare elapsed time against a timeout, use
if (System.nanoTime() - startTime >= timeoutNanos) ...
instead of
if (System.nanoTime() >= startTime + timeoutNanos) ...
because of the possibility of numerical overflow.
Is that of concern in this case?
On 14 Mar 2019, at 19:49, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote:
Hello!
The default implementation of Process.waitFor(long, TimeUnit) does not check if
the process has exited after the last portion of the timeout has expired.
JDK has two implementations of Process (for Unix and Windows) and they both
override waitFor(), so it's not an issue for them.
Still, it is better to provide a more accurate default implementation.
I'm not quite certain the regression test needs to be included in the fix. The
test does demonstrate the issue with the unfixed JDK and passed Okay on all
tested platforms in Mach5. Yet, I suspect the test can still show false
negative results, as there are no guaranties that even such simple application
as `true` will finish in 100 ms.
I can tag the test as @ignored with a comment, or simply remove it from the fix.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8220684
WEBREV: http://cr.openjdk.java.net/~igerasim/8220684/00/webrev/
Thanks in advance for reviewing!
--
With kind regards,
Ivan Gerasimov
--
With kind regards,
Ivan Gerasimov
