-
Bug
-
Resolution: Fixed
-
P4
-
None
-
b13
-
Verified
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 and they both override waitFor(), so it's not an issue for them.
Still, it is better to provide a more accurate default implementation.
The issue can be demonstrated with the following code:
--------------------
import java.io.InputStream;
import java.io.OutputStream;
import java.util.concurrent.TimeUnit;
public class T {
public static void main(String[] args) throws Throwable {
for (int i = 0; i < 100; ++i) {
Process proc = new MyProcess(new ProcessBuilder("true").start());
long start = System.nanoTime();
boolean exited = proc.waitFor(100, TimeUnit.MILLISECONDS);
if (!exited)
System.out.println(proc.pid() + " is alive after " +
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start) +
" ms.");
}
}
}
class MyProcess extends Process {
Process impl;
public MyProcess(Process impl) { this.impl = impl; }
public OutputStream getOutputStream() { return impl.getOutputStream(); }
public InputStream getInputStream() { return impl.getInputStream(); }
public InputStream getErrorStream() { return impl.getErrorStream(); }
public int waitFor() throws InterruptedException { return impl.waitFor(); }
// public boolean waitFor(long timeout, TimeUnit unit) throws InterruptedException
public int exitValue() { return impl.exitValue(); }
public void destroy() { impl.destroy(); }
public ProcessHandle toHandle() { return impl.toHandle(); }
}
----------------
With current JDK the code above reports a few dozens of "still alive" processes, which have in fact exited.
With the fix in place, no such processes are reported.
JDK has two implementations of Process and they both override waitFor(), so it's not an issue for them.
Still, it is better to provide a more accurate default implementation.
The issue can be demonstrated with the following code:
--------------------
import java.io.InputStream;
import java.io.OutputStream;
import java.util.concurrent.TimeUnit;
public class T {
public static void main(String[] args) throws Throwable {
for (int i = 0; i < 100; ++i) {
Process proc = new MyProcess(new ProcessBuilder("true").start());
long start = System.nanoTime();
boolean exited = proc.waitFor(100, TimeUnit.MILLISECONDS);
if (!exited)
System.out.println(proc.pid() + " is alive after " +
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start) +
" ms.");
}
}
}
class MyProcess extends Process {
Process impl;
public MyProcess(Process impl) { this.impl = impl; }
public OutputStream getOutputStream() { return impl.getOutputStream(); }
public InputStream getInputStream() { return impl.getInputStream(); }
public InputStream getErrorStream() { return impl.getErrorStream(); }
public int waitFor() throws InterruptedException { return impl.waitFor(); }
// public boolean waitFor(long timeout, TimeUnit unit) throws InterruptedException
public int exitValue() { return impl.exitValue(); }
public void destroy() { impl.destroy(); }
public ProcessHandle toHandle() { return impl.toHandle(); }
}
----------------
With current JDK the code above reports a few dozens of "still alive" processes, which have in fact exited.
With the fix in place, no such processes are reported.