8143876: test/java/lang/ProcessHandle/TreeTest.java failed intermittently with assertion error

The parent pid may be re-used, check that the child was started after the parent

Reviewed-by: darcy
This commit is contained in:
Roger Riggs 2015-12-02 09:40:14 -05:00
parent 7010cf730d
commit bdca5d5f82
3 changed files with 40 additions and 6 deletions

View File

@ -359,7 +359,14 @@ final class ProcessHandleImpl implements ProcessHandle {
@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 @@ final class ProcessHandleImpl implements ProcessHandle {
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 @@ final class ProcessHandleImpl implements ProcessHandle {
}
}
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;

View File

@ -184,7 +184,14 @@ Java_java_lang_ProcessHandleImpl_parent0(JNIEnv *env,
// 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));

View File

@ -29,6 +29,7 @@ import java.time.Instant;
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 @@ public class TreeTest extends ProcessUtil {
// 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 @@ public class TreeTest extends ProcessUtil {
// 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);