8176272: (process) ProcessHandle::onExit fails to wait for non-child process
authorrriggs
Thu, 16 Mar 2017 15:40:38 -0400
changeset 44269 762b2df849b3
parent 44268 1ec855c8a7d4
child 44270 46f8d5c5a6e5
8176272: (process) ProcessHandle::onExit fails to wait for non-child process Reviewed-by: chegar, stuefe
jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c
jdk/test/java/lang/ProcessHandle/JavaChild.java
jdk/test/java/lang/ProcessHandle/OnExitTest.java
--- a/jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java	Thu Mar 16 17:55:22 2017 +0000
+++ b/jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java	Thu Mar 16 15:40:38 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
  */
 package java.lang;
 
+import java.lang.annotation.Native;
 import java.security.PrivilegedAction;
 import java.time.Duration;
 import java.time.Instant;
@@ -57,6 +58,12 @@
     private static long REAPER_DEFAULT_STACKSIZE = 128 * 1024;
 
     /**
+     * Return value from waitForProcessExit0 indicating the process is not a child.
+     */
+    @Native
+    private static final int NOT_A_CHILD = -2;
+
+    /**
      * Cache the ProcessHandle of this process.
      */
     private static final ProcessHandleImpl current;
@@ -131,6 +138,29 @@
                 // spawn a thread to wait for and deliver the exit value
                 processReaperExecutor.execute(() -> {
                     int exitValue = waitForProcessExit0(pid, shouldReap);
+                    if (exitValue == NOT_A_CHILD) {
+                        // pid not alive or not a child of this process
+                        // If it is alive wait for it to terminate
+                        long sleep = 300;     // initial milliseconds to sleep
+                        int incr = 30;        // increment to the sleep time
+
+                        long startTime = isAlive0(pid);
+                        long origStart = startTime;
+                        while (startTime >= 0) {
+                            try {
+                                Thread.sleep(Math.min(sleep, 5000L)); // no more than 5 sec
+                                sleep += incr;
+                            } catch (InterruptedException ie) {
+                                // ignore and retry
+                            }
+                            startTime = isAlive0(pid);  // recheck if is alive
+                            if (origStart > 0 && startTime != origStart) {
+                                // start time changed, pid is not the same process
+                                break;
+                            }
+                        }
+                        exitValue = 0;
+                    }
                     newCompletion.complete(exitValue);
                     // remove from cache afterwards
                     completions.remove(pid, newCompletion);
--- a/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c	Thu Mar 16 17:55:22 2017 +0000
+++ b/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c	Thu Mar 16 15:40:38 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -244,7 +244,8 @@
         int status;
         while (waitpid(pid, &status, 0) < 0) {
             switch (errno) {
-                case ECHILD: return 0;
+                case ECHILD:
+                    return java_lang_ProcessHandleImpl_NOT_A_CHILD; // No child
                 case EINTR: break;
                 default: return -1;
             }
@@ -269,9 +270,10 @@
         memset(&siginfo, 0, sizeof siginfo);
         while (waitid(P_PID, pid, &siginfo, options) < 0) {
             switch (errno) {
-            case ECHILD: return 0;
-            case EINTR: break;
-            default: return -1;
+                case ECHILD:
+                    return java_lang_ProcessHandleImpl_NOT_A_CHILD; // No child
+                case EINTR: break;
+                default: return -1;
             }
         }
 
--- a/jdk/test/java/lang/ProcessHandle/JavaChild.java	Thu Mar 16 17:55:22 2017 +0000
+++ b/jdk/test/java/lang/ProcessHandle/JavaChild.java	Thu Mar 16 15:40:38 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,7 +28,6 @@
 import java.io.InputStreamReader;
 import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.PrintStream;
 import java.io.Reader;
 import java.io.PrintWriter;
 import java.lang.InterruptedException;
@@ -39,9 +38,11 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.Optional;
 import java.util.function.Consumer;
 
 
@@ -437,6 +438,11 @@
                     case "threaddump":
                         Thread.dumpStack();
                         break;
+                    case "waitpid":
+                        long pid = Long.parseLong(args[nextArg++]);
+                        Optional<String> s = ProcessHandle.of(pid).map(ph -> waitAlive(ph));
+                        sendResult(action, s.orElse("pid not valid: " + pid));
+                        break;
                     default:
                         throw new Error("JavaChild action unknown: " + action);
                 }
@@ -447,6 +453,17 @@
         }
     }
 
+    private static String waitAlive(ProcessHandle ph) {
+        String status;
+        try {
+            boolean isAlive = ph.onExit().get().isAlive();
+            status = Boolean.toString(isAlive);
+        } catch (InterruptedException | ExecutionException ex ) {
+            status = "interrupted";
+        }
+        return status;
+    }
+
     static synchronized void sendRaw(String s) {
         System.out.println(s);
         System.out.flush();
@@ -476,6 +493,7 @@
         System.err.println("  spawn <n> command... - spawn n new children and send command");
         System.err.println("  child command... - send command to all live children");
         System.err.println("  child_eof - send eof to all live children");
+        System.err.println("  waitpid <pid> - wait for the pid to exit");
         System.err.println("  exit <exitcode>");
         System.err.println("  out arg...");
         System.err.println("  err arg...");
--- a/jdk/test/java/lang/ProcessHandle/OnExitTest.java	Thu Mar 16 17:55:22 2017 +0000
+++ b/jdk/test/java/lang/ProcessHandle/OnExitTest.java	Thu Mar 16 15:40:38 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,9 +26,11 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 
 import jdk.test.lib.Utils;
 
@@ -196,4 +198,95 @@
         }
     }
 
+    /**
+     * Verify that onExit completes for a non-child process only when
+     * the process has exited.
+     * Spawn a child (A) waiting to be commanded to exit.
+     * Spawn a child (B) to wait for that process to exit.
+     * Command (A) to exit.
+     * Check that (B) does not complete until (A) has exited.
+     */
+    @Test
+    public static void peerOnExitTest() {
+        String line = null;
+        ArrayBlockingQueue<String> alines = new ArrayBlockingQueue<>(100);
+        ArrayBlockingQueue<String> blines = new ArrayBlockingQueue<>(100);
+        JavaChild A = null;
+        try {
+            String[] split;
+            A = JavaChild.spawnJavaChild("stdin");
+            A.forEachOutputLine(l -> alines.add(l));
+
+            // Verify A is running
+            A.sendAction("pid");
+            do {
+                split = getSplitLine(alines);
+            } while (!"pid".equals(split[1]));
+
+            JavaChild B = null;
+            try {
+                B = JavaChild.spawnJavaChild("stdin");
+                B.forEachOutputLine(l -> blines.add(l));
+
+                // Verify B is running
+                B.sendAction("pid");
+                do {
+                    split = getSplitLine(blines);
+                } while (!"pid".equals(split[1]));
+
+                // Tell B to wait for A's pid
+                B.sendAction("waitpid", A.getPid());
+
+                // Wait a bit to see if B will prematurely report the termination of A
+                try {
+                    line = blines.poll(5L, TimeUnit.SECONDS);
+                } catch (InterruptedException ie) {
+                    Assert.fail("interrupted", ie);
+                }
+                Assert.assertNull(line, "waitpid didn't wait");
+
+                A.sendAction("exit", 0L);
+
+                // Look for B to report that A has exited
+                do {
+                    split = getSplitLine(blines);
+                } while (!"waitpid".equals(split[1]));
+
+                Assert.assertEquals(split[2], "false",  "Process A should not be alive");
+
+                B.sendAction("exit", 0L);
+            } catch (IOException ioe) {
+                Assert.fail("unable to start JavaChild B", ioe);
+            } finally {
+                B.destroyForcibly();
+            }
+        } catch (IOException ioe2) {
+            Assert.fail("unable to start JavaChild A", ioe2);
+        } finally {
+            A.destroyForcibly();
+        }
+    }
+
+    private static boolean DEBUG = true;
+
+    /**
+     * Get a line from the queue and split into words on whitespace.
+     * Log to stdout if requested.
+     * @param queue a queue of strings
+     * @return the words split from the line.
+     */
+    private static String[] getSplitLine(ArrayBlockingQueue<String> queue) {
+        try {
+            String line = queue.take();
+            String[] split = line.split("\\s");
+            if (DEBUG) {
+                System.out.printf("  Child Output: %s%n", line);
+            }
+            return split;
+        } catch (InterruptedException ie) {
+            Assert.fail("interrupted", ie);
+            return null;
+        }
+    }
+
 }