8176272: (process) ProcessHandle::onExit fails to wait for non-child process
Reviewed-by: chegar, stuefe
--- 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;
+ }
+ }
+
}