8143876: test/java/lang/ProcessHandle/TreeTest.java failed intermittently with assertion error
Summary: The parent pid may be re-used, check that the child was started after the parent
Reviewed-by: darcy
--- a/jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java Wed Dec 02 12:28:24 2015 +0100
+++ b/jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java Wed Dec 02 09:40:14 2015 -0500
@@ -359,7 +359,14 @@
@Override
public Stream<ProcessHandle> children() {
- return children(pid);
+ // The native OS code selects based on matching the requested parent pid.
+ // If the original parent exits, the pid may have been re-used for
+ // this newer process.
+ // Processes started by the original parent (now dead) will all have
+ // start times less than the start of this newer parent.
+ // Processes started by this newer parent will have start times equal
+ // or after this parent.
+ return children(pid).filter(ph -> startTime <= ((ProcessHandleImpl)ph).startTime);
}
/**
@@ -408,11 +415,21 @@
int next = 0; // index of next process to check
int count = -1; // count of subprocesses scanned
long ppid = pid; // start looking for this parent
+ long ppStart = 0;
+ // Find the start time of the parent
+ for (int i = 0; i < size; i++) {
+ if (pids[i] == ppid) {
+ ppStart = starttimes[i];
+ break;
+ }
+ }
do {
- // Scan from next to size looking for ppid
- // if found, exchange it to index next
+ // Scan from next to size looking for ppid with child start time
+ // the same or later than the parent.
+ // If found, exchange it with index next
for (int i = next; i < size; i++) {
- if (ppids[i] == ppid) {
+ if (ppids[i] == ppid &&
+ ppStart <= starttimes[i]) {
swap(pids, i, next);
swap(ppids, i, next);
swap(starttimes, i, next);
@@ -420,6 +437,7 @@
}
}
ppid = pids[++count]; // pick up the next pid to scan for
+ ppStart = starttimes[count]; // and its start time
} while (count < next);
final long[] cpids = pids;
--- a/jdk/src/java.base/windows/native/libjava/ProcessHandleImpl_win.c Wed Dec 02 12:28:24 2015 +0100
+++ b/jdk/src/java.base/windows/native/libjava/ProcessHandleImpl_win.c Wed Dec 02 09:40:14 2015 -0500
@@ -184,7 +184,14 @@
// Now walk the snapshot of processes, and
do {
if (wpid == pe32.th32ProcessID) {
- ppid = pe32.th32ParentProcessID;
+ // The parent PID may be stale if that process has exited
+ // and may have been reused.
+ // A valid parent's start time is the same or before the child's
+ jlong ppStartTime = Java_java_lang_ProcessHandleImpl_isAlive0(env,
+ clazz, pe32.th32ParentProcessID);
+ if (ppStartTime > 0 && ppStartTime <= startTime) {
+ ppid = pe32.th32ParentProcessID;
+ }
break;
}
} while (Process32Next(hProcessSnap, &pe32));
--- a/jdk/test/java/lang/ProcessHandle/TreeTest.java Wed Dec 02 12:28:24 2015 +0100
+++ b/jdk/test/java/lang/ProcessHandle/TreeTest.java Wed Dec 02 09:40:14 2015 -0500
@@ -29,6 +29,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
@@ -168,8 +169,16 @@
// Wait for direct children to be created and save the list
List<ProcessHandle> subprocesses = waitForAllChildren(p1Handle, spawnNew);
+ Optional<Instant> p1Start = p1Handle.info().startInstant();
for (ProcessHandle ph : subprocesses) {
Assert.assertTrue(ph.isAlive(), "Child should be alive: " + ph);
+ // Verify each child was started after the parent
+ ph.info().startInstant()
+ .ifPresent(childStart -> p1Start.ifPresent(parentStart -> {
+ Assert.assertFalse(childStart.isBefore(parentStart),
+ String.format("Child process started before parent: child: %s, parent: %s",
+ childStart, parentStart));
+ }));
}
// Each child spawns two processes and waits for commands
@@ -178,7 +187,7 @@
// Poll until all 9 child processes exist or the timeout is reached
int expected = 9;
- long timeout = Utils.adjustTimeout(10L);
+ long timeout = Utils.adjustTimeout(60L);
Instant endTimeout = Instant.now().plusSeconds(timeout);
do {
Thread.sleep(200L);