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

3 files changed, 16 insertions, 2 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
index 107f620c26..383b5c8ad1 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
@@ -44,7 +44,7 @@ public class WindowsFileOperationsTest {
 
   @Before
   public void loadJni() throws Exception {
-    scratchRoot = new File(System.getenv("TEST_TMPDIR")).getAbsolutePath() + "/x";
+    scratchRoot = new File(System.getenv("TEST_TMPDIR"), "x").getAbsolutePath();
     testUtil = new WindowsTestUtil(scratchRoot);
     cleanupScratchDir();
   }
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
index 7930957680..e52c9b65bc 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
@@ -56,7 +56,7 @@ public class WindowsFileSystemTest {
 
   @Before
   public void loadJni() throws Exception {
-    scratchRoot = new File(System.getenv("TEST_TMPDIR")).getAbsolutePath() + "/x";
+    scratchRoot = new File(System.getenv("TEST_TMPDIR"), "x").getAbsolutePath();
     testUtil = new WindowsTestUtil(scratchRoot);
     fs = new WindowsFileSystem();
     cleanupScratchDir();
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
index af5244d6b0..290960bc68 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
@@ -357,6 +357,20 @@ public class WindowsProcessesTest {
   }
 
   @Test
+  public void testRedirectedErrorStream() throws Exception {
+    process =
+        WindowsProcesses.createProcess(
+            mockBinary, mockArgs("O-one", "E-two"), null, null, null, null, true);
+    assertNoProcessError();
+    byte[] buf = new byte[6];
+    assertThat(readStdout(buf, 0, 3)).isEqualTo(3);
+    assertThat(readStdout(buf, 3, 3)).isEqualTo(3);
+    assertThat(new String(buf, UTF8)).isEqualTo("onetwo");
+    assertThat(readStderr(buf, 0, 1)).isEqualTo(0);
+    WindowsProcesses.waitFor(process, -1);
+  }
+
+  @Test
   public void testAppendToExistingFile() throws Exception {
     String stdoutFile = System.getenv("TEST_TMPDIR") + "\\stdout_atef";
     String stderrFile = System.getenv("TEST_TMPDIR") + "\\stderr_atef";