Mercurial Mercurial > jdk-sandbox / changeset
summary | shortlog | changelog | graph | tags | bookmarks | branches | files | changeset | raw | help
Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8067796: (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null
authorrriggs
Mon, 23 Mar 2015 10:13:32 -0400
changeset 29604 f3e313c492ba
parent 29603 8bdfa87650e4
child 29612 ab106d3c4e68
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
jdk/src/java.base/unix/classes/java/lang/ProcessImpl.java file | annotate | diff | comparison | revisions
jdk/src/java.base/windows/classes/java/lang/ProcessImpl.java file | annotate | diff | comparison | revisions
jdk/test/java/lang/ProcessBuilder/Basic.java file | annotate | diff | comparison | revisions 
--- a/jdk/src/java.base/unix/classes/java/lang/ProcessImpl.java	Mon Mar 23 09:45:32 2015 -0700
+++ b/jdk/src/java.base/unix/classes/java/lang/ProcessImpl.java	Mon Mar 23 10:13:32 2015 -0400
@@ -496,12 +496,11 @@
     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 {
             // Round up to next millisecond
             wait(TimeUnit.NANOSECONDS.toMillis(remainingNanos + 999_999L));
--- a/jdk/src/java.base/windows/classes/java/lang/ProcessImpl.java	Mon Mar 23 09:45:32 2015 -0700
+++ b/jdk/src/java.base/windows/classes/java/lang/ProcessImpl.java	Mon Mar 23 10:13:32 2015 -0400
@@ -458,12 +458,11 @@
     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);
--- a/jdk/test/java/lang/ProcessBuilder/Basic.java	Mon Mar 23 09:45:32 2015 -0700
+++ b/jdk/test/java/lang/ProcessBuilder/Basic.java	Mon Mar 23 10:13:32 2015 -0400
@@ -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 8003488 8054494 8058464
+ *      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
@@ -2387,6 +2388,56 @@
         } catch (Throwable t) { unexpected(t); }
 
         //----------------------------------------------------------------
+        // Check that Process.waitFor(timeout, null) throws NPE.
+        //----------------------------------------------------------------
+        try {
+            List<String> childArgs = new ArrayList<String>(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<String> childArgs = new ArrayList<String>(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)
         //----------------------------------------------------------------
jdk-sandbox