8272600: (test) Use native "sleep" in Basic.java

Reviewed-by: iklam, dholmes

Author: Roger Riggs

test/jdk/java/lang/ProcessBuilder/Basic.java

@@ -27,13 +27,13 @@
  *      5026830 5023243 5070673 4052517 4811767 6192449 6397034 6413313
  *      6464154 6523983 6206031 4960438 6631352 6631966 6850957 6850958
  *      4947220 7018606 7034570 4244896 5049299 8003488 8054494 8058464
- *      8067796 8224905 8263729 8265173
+ *      8067796 8224905 8263729 8265173 8272600 8231297
  * @key intermittent
  * @summary Basic tests for Process and Environment Variable code
  * @modules java.base/java.lang:open
  * @library /test/lib
- * @run main/othervm/timeout=300 -Djava.security.manager=allow Basic
- * @run main/othervm/timeout=300 -Djava.security.manager=allow -Djdk.lang.Process.launchMechanism=fork Basic
+ * @run main/othervm/native/timeout=300 -Djava.security.manager=allow Basic
+ * @run main/othervm/native/timeout=300 -Djava.security.manager=allow -Djdk.lang.Process.launchMechanism=fork Basic
  * @author Martin Buchholz
  */
 
@@ -50,8 +50,8 @@
 import static java.lang.ProcessBuilder.Redirect.*;
 
 import java.io.*;
-import java.lang.reflect.Field;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.*;
@@ -85,7 +85,7 @@ public class Basic {
     /**
      * Returns the number of milliseconds since time given by
      * startNanoTime, which must have been previously returned from a
-     * call to {@link System.nanoTime()}.
+     * call to {@link System#nanoTime()}.
      */
     private static long millisElapsedSince(long startNanoTime) {
         return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanoTime);
@@ -2137,34 +2137,8 @@ public void doIt(Map<String,String> environ) {
             final int cases = 4;
             for (int i = 0; i < cases; i++) {
                 final int action = i;
-                List<String> childArgs = new ArrayList<>(javaChildArgs);
+                List<String> childArgs = getSleepArgs();
                 final ProcessBuilder pb = new ProcessBuilder(childArgs);
-                {
-                    // Redirect any child VM error output away from the stream being tested
-                    // and to the log file. For background see:
-                    // 8231297: java/lang/ProcessBuilder/Basic.java test fails intermittently
-                    // Destroying the process may, depending on the timing, cause some output
-                    // from the child VM.
-                    // This test requires the thread reading from the subprocess be blocked
-                    // in the read from the subprocess; there should be no bytes to read.
-                    // Modify the argument list shared with ProcessBuilder to redirect VM output.
-                    assert (childArgs.get(1).equals("-XX:+DisplayVMOutputToStderr")) : "Expected arg 1 to be \"-XX:+DisplayVMOutputToStderr\"";
-                    switch (action & 0x1) {
-                        case 0:
-                            childArgs.set(1, "-XX:+DisplayVMOutputToStderr");
-                            childArgs.add(2, "-Xlog:all=warning:stderr");
-                            pb.redirectError(INHERIT);
-                            break;
-                        case 1:
-                            childArgs.set(1, "-XX:+DisplayVMOutputToStdout");
-                            childArgs.add(2, "-Xlog:all=warning:stdout");
-                            pb.redirectOutput(INHERIT);
-                            break;
-                        default:
-                            throw new Error();
-                    }
-                }
-                childArgs.add("sleep");
                 final byte[] bytes = new byte[10];
                 final Process p = pb.start();
                 final CountDownLatch latch = new CountDownLatch(1);
@@ -2237,9 +2211,10 @@ && new File("/bin/sleep").exists()) {
                 // our child) but not our grandchild (i.e. '/bin/sleep'). So
                 // pay attention that the grandchild doesn't run too long to
                 // avoid polluting the process space with useless processes.
-                // Running the grandchild for 60s should be more than enough.
-                final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep 60)" };
-                final String[] cmdkill = { "/bin/bash", "-c", "(/usr/bin/pkill -f \"sleep 60\")" };
+                // Running the grandchild for 59s should be more than enough.
+                // A unique (59s) time is needed to avoid killing other sleep processes.
+                final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep 59)" };
+                final String[] cmdkill = { "/bin/bash", "-c", "(/usr/bin/pkill -f \"sleep 59\")" };
                 final ProcessBuilder pb = new ProcessBuilder(cmd);
                 final Process p = pb.start();
                 final InputStream stdout = p.getInputStream();
@@ -2441,8 +2416,7 @@ public void run() {
         // Process.waitFor(0, TimeUnit.MILLISECONDS) work as expected.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             long start = System.nanoTime();
             if (!p.isAlive() || p.waitFor(0, TimeUnit.MILLISECONDS)) {
@@ -2471,17 +2445,19 @@ public void run() {
         // works as expected.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             long start = System.nanoTime();
 
-            p.waitFor(10, TimeUnit.MILLISECONDS);
-
-            long end = System.nanoTime();
-            if ((end - start) < TimeUnit.MILLISECONDS.toNanos(10))
-                fail("Test failed: waitFor didn't take long enough (" + (end - start) + "ns)");
-
+            if (p.waitFor(10, TimeUnit.MILLISECONDS)) {
+                var msg = "External sleep process terminated early: exitValue: %d, (%dns)%n"
+                        .formatted(p.exitValue(), (System.nanoTime() - start));
+                fail(msg);
+            } else {
+                long end = System.nanoTime();
+                if ((end - start) < TimeUnit.MILLISECONDS.toNanos(10))
+                    fail("Test failed: waitFor didn't take long enough (" + (end - start) + "ns)");
+            }
             p.destroy();
         } catch (Throwable t) { unexpected(t); }
 
@@ -2490,8 +2466,7 @@ public void run() {
         // interrupt works as expected, if interrupted while waiting.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             final long start = System.nanoTime();
             final CountDownLatch aboutToWaitFor = new CountDownLatch(1);
@@ -2522,8 +2497,7 @@ public void run() {
         // interrupt works as expected, if interrupted while waiting.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             final long start = System.nanoTime();
             final CountDownLatch aboutToWaitFor = new CountDownLatch(1);
@@ -2554,8 +2528,7 @@ public void run() {
         // interrupt works as expected, if interrupted before waiting.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             final long start = System.nanoTime();
             final CountDownLatch threadStarted = new CountDownLatch(1);
@@ -2586,8 +2559,7 @@ public void run() {
         // Check that Process.waitFor(timeout, null) throws NPE.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process p = new ProcessBuilder(childArgs).start();
             THROWS(NullPointerException.class,
                     () ->  p.waitFor(10L, null));
@@ -2610,8 +2582,7 @@ public void run() {
         // Check that default implementation of Process.waitFor(timeout, null) throws NPE.
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process proc = new ProcessBuilder(childArgs).start();
             final DelegatingProcess p = new DelegatingProcess(proc);
 
@@ -2637,24 +2608,75 @@ public void run() {
         // Process.waitFor(long, TimeUnit)
         //----------------------------------------------------------------
         try {
-            List<String> childArgs = new ArrayList<String>(javaChildArgs);
-            childArgs.add("sleep");
+            List<String> childArgs = getSleepArgs();
             final Process proc = new ProcessBuilder(childArgs).start();
             DelegatingProcess p = new DelegatingProcess(proc);
             long start = System.nanoTime();
 
-            p.waitFor(1000, TimeUnit.MILLISECONDS);
-
-            long end = System.nanoTime();
-            if ((end - start) < 500000000)
-                fail("Test failed: waitFor didn't take long enough");
-
+            if (p.waitFor(1000, TimeUnit.MILLISECONDS)) {
+                var msg = "External sleep process terminated early: exitValue: %02x, (%dns)"
+                        .formatted(p.exitValue(), (System.nanoTime() - start));
+                fail(msg);
+            } else {
+                long end = System.nanoTime();
+                if ((end - start) < 500000000)
+                    fail("Test failed: waitFor didn't take long enough (" + (end - start) + "ns)");
+            }
             p.destroy();
 
             p.waitFor(1000, TimeUnit.MILLISECONDS);
         } catch (Throwable t) { unexpected(t); }
     }
 
+    // Path to native executables, if any
+    private static final String TEST_NATIVEPATH = System.getProperty("test.nativepath");
+
+    // Path where "sleep" program may be found" or null
+    private static final Path SLEEP_PATH = initSleepPath();
+
+    /**
+     * Compute the Path to a sleep executable.
+     * @return a Path to sleep or BasicSleep(.exe) or null if none
+     */
+    private static Path initSleepPath() {
+        if (Windows.is() && TEST_NATIVEPATH != null) {
+            // exeBasicSleep is equivalent to sleep on Unix
+            Path exePath = Path.of(TEST_NATIVEPATH).resolve("BasicSleep.exe");
+            if (Files.isExecutable(exePath)) {
+                return exePath;
+            }
+        }
+
+        List<String> binPaths = List.of("/bin", "/usr/bin");
+        for (String dir : binPaths) {
+            Path exePath = Path.of(dir).resolve("sleep");
+            if (Files.isExecutable(exePath)) {
+                return exePath;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Return the list of process arguments for a child to sleep 10 minutes (600 seconds).
+     *
+     * @return A list of process arguments to sleep 10 minutes.
+     */
+    private static List<String> getSleepArgs() {
+        List<String> childArgs = null;
+        if (SLEEP_PATH != null) {
+            childArgs = List.of(SLEEP_PATH.toString(), "600");
+        } else {
+            // Fallback to the JavaChild ; its 'sleep' command is for 10 minutes.
+            // The fallback the Java$Child is used if the test is run without building
+            // the BasicSleep native executable (for Windows).
+            childArgs = new ArrayList<>(javaChildArgs);
+            childArgs.add("sleep");
+            System.out.println("Sleep not found, fallback to JavaChild: " + childArgs);
+        }
+        return childArgs;
+    }
+
     static void closeStreams(Process p) {
         try {
             p.getOutputStream().close();

test/jdk/java/lang/ProcessBuilder/exeBasicSleep.c

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2021, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef _WIN32
+    #include <windows.h>
+#else
+    #include <unistd.h>
+#endif
+/**
+ * Command line program to sleep at least given number of seconds.
+ * The behavior should equivalent to the Unix sleep command.
+ * Actual time sleeping may vary if interrupted, the remaining time
+ * returned from sleep has limited accuracy.
+ *
+ * Note: the file name prefix "exe" identifies the source should be built into BasicSleep(.exe).
+ */
+int main(int argc, char** argv) {
+    int seconds;
+
+    if (argc < 2 || (seconds = atoi(argv[1])) < 0) {
+        fprintf(stderr, "usage: BasicSleep <non-negative seconds>\n");
+        exit(1);
+    }
+
+#ifdef _WIN32
+    Sleep(seconds * 1000);
+#else
+    while ((seconds = sleep(seconds)) > 0) {
+        // until no more to sleep
+    }
+#endif
+}