From 3c74f3cc8b6679225cbb8ebdf82eb894f7758124 Mon Sep 17 00:00:00 2001 From: snazarki Date: Mon, 23 Mar 2015 10:13:32 -0400 Subject: [PATCH] 8067796: (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null Summary: Implement checking for NPE in Process implementation before other conditions Reviewed-by: martin, chegar, aph, andrew --- .../classes/java/lang/UNIXProcess.java | 3 +- .../classes/java/lang/ProcessImpl.java | 3 +- test/java/lang/ProcessBuilder/Basic.java | 51 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/solaris/classes/java/lang/UNIXProcess.java b/src/solaris/classes/java/lang/UNIXProcess.java index fe26fd893..674681cd2 100644 --- a/src/solaris/classes/java/lang/UNIXProcess.java +++ b/src/solaris/classes/java/lang/UNIXProcess.java @@ -401,12 +401,11 @@ final class UNIXProcess extends Process { public synchronized boolean waitFor(long timeout, TimeUnit unit) throws InterruptedException { + long remainingNanos = unit.toNanos(timeout); // throw NPE before other conditions if (hasExited) return true; if (timeout <= 0) return false; - long remainingNanos = unit.toNanos(timeout); long deadline = System.nanoTime() + remainingNanos; - do { TimeUnit.NANOSECONDS.timedWait(this, remainingNanos); if (hasExited) { diff --git a/src/windows/classes/java/lang/ProcessImpl.java b/src/windows/classes/java/lang/ProcessImpl.java index 1970260e5..5c43ef77e 100644 --- a/src/windows/classes/java/lang/ProcessImpl.java +++ b/src/windows/classes/java/lang/ProcessImpl.java @@ -516,12 +516,11 @@ final class ProcessImpl extends Process { public boolean waitFor(long timeout, TimeUnit unit) throws InterruptedException { + long remainingNanos = unit.toNanos(timeout); // throw NPE before other conditions if (getExitCodeProcess(handle) != STILL_ACTIVE) return true; if (timeout <= 0) return false; - long remainingNanos = unit.toNanos(timeout); long deadline = System.nanoTime() + remainingNanos; - do { // Round up to next millisecond long msTimeout = TimeUnit.NANOSECONDS.toMillis(remainingNanos + 999_999L); diff --git a/test/java/lang/ProcessBuilder/Basic.java b/test/java/lang/ProcessBuilder/Basic.java index 138199196..c8f2461c7 100644 --- a/test/java/lang/ProcessBuilder/Basic.java +++ b/test/java/lang/ProcessBuilder/Basic.java @@ -27,6 +27,7 @@ * 5026830 5023243 5070673 4052517 4811767 6192449 6397034 6413313 * 6464154 6523983 6206031 4960438 6631352 6631966 6850957 6850958 * 4947220 7018606 7034570 4244896 5049299 + * 8067796 * @summary Basic tests for Process and Environment Variable code * @run main/othervm/timeout=300 Basic * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=fork Basic @@ -2366,6 +2367,56 @@ public class Basic { p.destroy(); } catch (Throwable t) { unexpected(t); } + //---------------------------------------------------------------- + // Check that Process.waitFor(timeout, null) throws NPE. + //---------------------------------------------------------------- + try { + List childArgs = new ArrayList(javaChildArgs); + childArgs.add("sleep"); + final Process p = new ProcessBuilder(childArgs).start(); + THROWS(NullPointerException.class, + () -> p.waitFor(10L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(0L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(-1L, null)); + // Terminate process and recheck after it exits + p.destroy(); + p.waitFor(); + THROWS(NullPointerException.class, + () -> p.waitFor(10L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(0L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(-1L, null)); + } catch (Throwable t) { unexpected(t); } + + //---------------------------------------------------------------- + // Check that default implementation of Process.waitFor(timeout, null) throws NPE. + //---------------------------------------------------------------- + try { + List childArgs = new ArrayList(javaChildArgs); + childArgs.add("sleep"); + final Process proc = new ProcessBuilder(childArgs).start(); + final DelegatingProcess p = new DelegatingProcess(proc); + + THROWS(NullPointerException.class, + () -> p.waitFor(10L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(0L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(-1L, null)); + // Terminate process and recheck after it exits + p.destroy(); + p.waitFor(); + THROWS(NullPointerException.class, + () -> p.waitFor(10L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(0L, null)); + THROWS(NullPointerException.class, + () -> p.waitFor(-1L, null)); + } catch (Throwable t) { unexpected(t); } + //---------------------------------------------------------------- // Check the default implementation for // Process.waitFor(long, TimeUnit) -- GitLab