Windows, jni: Don't close stdout/stderr in nativeWaitFor function

These two close operations were added to work around #1708, but caused #2675.

We found the root cause of the hanging problem in #1708 is a race
condition when creating Windows processes:

When Bazel trys to create two processes, one for a local command
execution, one for starting the worker process. The worker process
might accidentally inherits handles opened when creating the local
command process, and it holds those handles as long as it lives.
Therefore, ReadFile function hangs when handles for the write end of
stdout/stderr pipes are released by the worker.

The solution is to make Bazel native createProcess JNI function
explicitly inheirts handles as needed, and use this function to start
worker process.

Related: http://support.microsoft.com/kb/315939
Fixed https://github.com/bazelbuild/bazel/issues/2675

Change-Id: I1c9b1ac3c9383ed2fd28ea92f528f19649693275
PiperOrigin-RevId: 173244832

2 files changed, 29 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
index f022ae47ff..a69884066c 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
@@ -52,7 +52,13 @@ public class WindowsSubprocessFactory implements SubprocessFactory {
 
     long nativeProcess =
         WindowsProcesses.createProcess(
-            argv0, argvRest, env, builder.getWorkingDirectory().getPath(), stdoutPath, stderrPath);
+            argv0,
+            argvRest,
+            env,
+            builder.getWorkingDirectory().getPath(),
+            stdoutPath,
+            stderrPath,
+            builder.redirectErrorStream());
     String error = WindowsProcesses.processGetLastError(nativeProcess);
     if (!error.isEmpty()) {
       WindowsProcesses.deleteProcess(nativeProcess);
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
index 1eaab25904..bb13941bcb 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
@@ -33,7 +33,6 @@ public class WindowsProcesses {
    *     Windows path with a drive letter (e.g. "c:\foo\bar app.exe") or a single file name (e.g.
    *     "foo app.exe")
    * @param argvRest the rest of the command line, i.e. argv[1:] (needs to be quoted Windows style)
-   * @param commandLine the command line (needs to be quoted Windows style)
    * @param env the environment of the new process. null means inherit that of the Bazel server
    * @param cwd the working directory of the new process. if null, the same as that of the current
    *     process
@@ -41,16 +40,36 @@ public class WindowsProcesses {
    *     work.
    * @param stderrFile the file the stdout should be redirected to. if null, nativeReadStderr will
    *     work.
+   * @param redirectErrorStream whether we merge the process's standard error and standard output.
    * @return the opaque identifier of the created process
    */
   public static long createProcess(
+      String argv0,
+      String argvRest,
+      byte[] env,
+      String cwd,
+      String stdoutFile,
+      String stderrFile,
+      boolean redirectErrorStream) {
+    WindowsJniLoader.loadJni();
+    return nativeCreateProcess(
+        argv0, argvRest, env, cwd, stdoutFile, stderrFile, redirectErrorStream);
+  }
+
+  public static long createProcess(
       String argv0, String argvRest, byte[] env, String cwd, String stdoutFile, String stderrFile) {
     WindowsJniLoader.loadJni();
-    return nativeCreateProcess(argv0, argvRest, env, cwd, stdoutFile, stderrFile);
+    return nativeCreateProcess(argv0, argvRest, env, cwd, stdoutFile, stderrFile, false);
   }
 
   private static native long nativeCreateProcess(
-      String argv0, String argvRest, byte[] env, String cwd, String stdoutFile, String stderrFile);
+      String argv0,
+      String argvRest,
+      byte[] env,
+      String cwd,
+      String stdoutFile,
+      String stderrFile,
+      boolean redirectErrorStream);
 
   /**
    * Writes data from the given array to the stdin of the specified process.