Bazel server: ensure InputStreams are closed

Use try-with-resources to ensure InputStreams that
we open via FileSystem.InputStream(path) are
closed.

Eagerly closing InputStreams avoids hanging on to
file handles until the garbage collector finalizes
the InputStream, meaning Bazel on Windows (and
other processes) can delete or mutate these files.

Hopefully this avoids intermittent file deletion
errors that sometimes occur on Windows.

See https://github.com/bazelbuild/bazel/issues/5512

RELNOTES: none
PiperOrigin-RevId: 203338148

3 files changed, 16 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
index c48674a913..2a54103fde 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
@@ -15,13 +15,14 @@ package com.google.devtools.build.lib.exec;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.io.ByteSource;
 import com.google.common.io.LineProcessor;
 import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -59,8 +60,13 @@ public final class FilesetManifest {
       RelativeSymlinkBehavior relSymlinkBehavior)
           throws IOException {
     Path file = execRoot.getRelative(AnalysisUtils.getManifestPathFromFilesetPath(manifest));
-    try {
-      return FileSystemUtils.asByteSource(file).asCharSource(UTF_8)
+    try (InputStream in = file.getInputStream()) {
+      return new ByteSource() {
+        @Override
+        public InputStream openStream() throws IOException {
+          return in;
+        }
+      }.asCharSource(UTF_8)
           .readLines(
               new ManifestLineProcessor(workspaceName, manifest, relSymlinkBehavior));
     } catch (IllegalStateException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestLogHelper.java b/src/main/java/com/google/devtools/build/lib/exec/TestLogHelper.java
index 65d1d61d15..409c10d903 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestLogHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestLogHelper.java
@@ -48,7 +48,6 @@ public class TestLogHelper {
    */
   public static void writeTestLog(Path testOutput, String testName, OutputStream out)
       throws IOException {
-    InputStream input = null;
     PrintStream printOut = new PrintStream(new BufferedOutputStream(out));
     try {
       final String outputHeader = "==================== Test output for " + testName + ":";
@@ -58,9 +57,10 @@ public class TestLogHelper {
       printOut.println(outputHeader);
       printOut.flush();
 
-      input = testOutput.getInputStream();
       FilterTestHeaderOutputStream filteringOutputStream = getHeaderFilteringOutputStream(printOut);
-      ByteStreams.copy(input, filteringOutputStream);
+      try (InputStream input = testOutput.getInputStream()) {
+        ByteStreams.copy(input, filteringOutputStream);
+      }
 
       if (!filteringOutputStream.foundHeader()) {
         try (InputStream inputAgain = testOutput.getInputStream()) {
@@ -71,9 +71,6 @@ public class TestLogHelper {
       printOut.println(outputFooter);
     } finally {
       printOut.flush();
-      if (input != null) {
-        input.close();
-      }
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 26a24b1e85..2088dabf5b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
-import com.google.common.io.Closeables;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
@@ -418,21 +417,18 @@ public abstract class TestStrategy implements TestActionContext {
   /** In rare cases, we might write something to stderr. Append it to the real test.log. */
   protected static void appendStderr(Path stdOut, Path stdErr) throws IOException {
     FileStatus stat = stdErr.statNullable();
-    OutputStream out = null;
-    InputStream in = null;
     if (stat != null) {
       try {
         if (stat.getSize() > 0) {
           if (stdOut.exists()) {
             stdOut.setWritable(true);
           }
-          out = stdOut.getOutputStream(true);
-          in = stdErr.getInputStream();
-          ByteStreams.copy(in, out);
+          try (OutputStream out = stdOut.getOutputStream(true);
+              InputStream in = stdErr.getInputStream()) {
+            ByteStreams.copy(in, out);
+          }
         }
       } finally {
-        Closeables.close(out, true);
-        Closeables.close(in, true);
         stdErr.delete();
       }
     }