8143876: test/java/lang/ProcessHandle/TreeTest.java failed intermittently with assertion error
authorrriggs
Wed, 02 Dec 2015 09:40:14 -0500
changeset 34385 0762c1d51f8a
parent 34384 439c06c76808
child 34386 80c21b82e467
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
jdk/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
jdk/src/java.base/windows/native/libjava/ProcessHandleImpl_win.c
jdk/test/java/lang/ProcessHandle/TreeTest.java
--- 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);