8377935: (process) jspawnhelper race can cause hangs in ProcessBuilder - fix for FORK mode

src/java.base/unix/native/libjava/ProcessImpl_md.c

@@ -36,15 +36,17 @@
  * Platform-specific support for java.lang.Process
  */
 #include <assert.h>
+#include <ctype.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <spawn.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/types.h>
-#include <ctype.h>
 #include <sys/wait.h>
-#include <signal.h>
-#include <string.h>
-
-#include <spawn.h>
+#include <unistd.h>
 
 #include "childproc.h"
 
@@ -666,6 +668,21 @@ startChild(JNIEnv *env, jobject process, ChildStuff *c, const char *helperpath)
     }
 }
 
+static int pipeOrPipe2(int fd[2], bool cloexec) {
+#ifdef HAVE_PIPE2
+    return pipe2(fd, cloexec ? O_CLOEXEC : 0);
+#else
+    /* Do the next best thing - pipe, but tag file descriptors right afterwards.
+     * Still racy, but dangerous time window is as short as we can make it. */
+    int rc = pipe(fd);
+    if (rc == 0 && cloexec) {
+        fcntl(fds[0], F_SETFD, FD_CLOEXEC);
+        fcntl(fds[0], F_SETFD, FD_CLOEXEC);
+    }
+    return rc;
+#endif /* HAVE_PIPE2 */
+}
+
 JNIEXPORT jint JNICALL
 Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
                                        jobject process,
@@ -727,15 +744,28 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
     fds = (*env)->GetIntArrayElements(env, std_fds, NULL);
     if (fds == NULL) goto Catch;
 
-    if ((fds[0] == -1 && pipe(in)  < 0) ||
-        (fds[1] == -1 && pipe(out) < 0) ||
-        (fds[2] == -1 && !redirectErrorStream && pipe(err) < 0) || // if not redirecting create the pipe
-        (pipe(childenv) < 0) ||
-        (pipe(fail) < 0)) {
+    /* In FORK/VFORK mode, we must ensure the pipe fd's are set to CLOEXEC as early as possible.
+     * That is because as long as we have the pipes open and not set to CLOEXEC, a native third-party
+     * thread doing a native fork() (uncontrolled by us) would carry a copy of the pipe fd's and
+     * possibly keep open the fail pipe we need to communicate from child to parent. That would cause
+     * parent to hang (see JDK-8377907).
+     * Note that setting the in/out/err pipes to CLOEXEC is fine, too, since the child process will
+     * dup2() those filedescriptors to their stdin/stdout/stderr. And dup2() does not copy the
+     * CLOEXEC flag. */
+    const bool cloExec = ((mode == MODE_FORK) || (mode == MODE_VFORK));
+    if ((fds[0] == -1 && pipeOrPipe2(in, cloExec)  < 0) ||
+        (fds[1] == -1 && pipeOrPipe2(out, cloExec) < 0) ||
+        (fds[2] == -1 && pipeOrPipe2(err, cloExec) < 0) ||
+        (pipeOrPipe2(childenv, cloExec) < 0) ||
+        (pipeOrPipe2(fail, cloExec) < 0)) {
         throwInternalIOException(env, errno, "Bad file descriptor", mode);
         goto Catch;
     }
 
+#ifdef DEBUG
+    jtregSimulateDelay();
+#endif
+
     c->fds[0] = fds[0];
     c->fds[1] = fds[1];
     c->fds[2] = fds[2];

src/java.base/unix/native/libjava/childproc.c

@@ -468,6 +468,15 @@ childProcess(void *arg)
 }
 
 #ifdef DEBUG
+/* This method is only used in debug builds */
+void jtregSimulateDelay() {
+    const char* env = getenv("JTREG_JSPAWNHELPER_DELAY_TEST");
+    if (env != NULL && atoi(env) == 1) {
+        printf("delay\n");
+        fflush(stdout);
+        sleep(1);
+    }
+}
 /* This method is only used in debug builds for testing MODE_POSIX_SPAWN
  * in the light of abnormal program termination of either the parent JVM
  * or the newly created jspawnhelper child process during the execution of

src/java.base/unix/native/libjava/childproc.h

@@ -136,6 +136,14 @@ int childProcess(void *arg);
  * See: test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
  */
 void jtregSimulateCrash(pid_t child, int stage);
+void jtregSimulateDelay();
+#endif
+
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__)
+#define HAVE_PIPE2
+#else
+// Neither MacOS nor AIX support pipe2, unfortunately
+#undef HAVE_PIPE2
 #endif
 
 #endif