changeset 13211:8aa69a089b8f

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
author rriggs
date Wed, 02 Dec 2015 09:40:14 -0500
parents 9c6b37a3b4a8
children 384f4b699387
files src/java.base/share/classes/java/lang/ProcessHandleImpl.java src/java.base/windows/native/libjava/ProcessHandleImpl_win.c test/java/lang/ProcessHandle/TreeTest.java
diffstat 3 files changed, 40 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/ProcessHandleImpl.java	Wed Dec 02 12:28:24 2015 +0100
+++ b/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/src/java.base/windows/native/libjava/ProcessHandleImpl_win.c	Wed Dec 02 12:28:24 2015 +0100
+++ b/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/test/java/lang/ProcessHandle/TreeTest.java	Wed Dec 02 12:28:24 2015 +0100
+++ b/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);