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

8 files changed, 80 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java
index ed1cdb17ae..d02c9cb4bb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java
@@ -75,8 +75,9 @@ public final class LauncherFileWriteAction extends AbstractFileWriteAction {
     // single-machine execution environment, but problematic with remote execution.
     Preconditions.checkState(OS.getCurrent() == OS.WINDOWS);
     return out -> {
-      InputStream in = ctx.getInputPath(this.launcher).getInputStream();
-      ByteStreams.copy(in, out);
+      try (InputStream in = ctx.getInputPath(this.launcher).getInputStream()) {
+        ByteStreams.copy(in, out);
+      }
       long dataLength = this.launchInfo.write(out);
       ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES);
       buffer.order(ByteOrder.LITTLE_ENDIAN); // All Windows versions are little endian.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
index ede9cd2620..46c42b606b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
@@ -46,6 +46,7 @@ import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.ValueOrException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.Map;
 import java.util.Properties;
 
@@ -311,9 +312,10 @@ public class AndroidSdkRepositoryFunction extends AndroidRepositoryFunction {
       env.getValueOrThrow(releaseFileKey, IOException.class);
 
       Properties properties = new Properties();
-      properties.load(sourcePropertiesFilePath.getInputStream());
+      try (InputStream in = sourcePropertiesFilePath.getInputStream()) {
+        properties.load(in);
+      }
       return properties;
-
     } catch (IOException e) {
       String error = String.format(
           "Could not read %s in Android SDK: %s", sourcePropertiesFilePath, e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/SdkMavenRepository.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/SdkMavenRepository.java
index 1efca1efde..97ebf914f8 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/SdkMavenRepository.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/SdkMavenRepository.java
@@ -21,6 +21,7 @@ import com.google.common.collect.Ordering;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -199,8 +200,10 @@ final class SdkMavenRepository {
     private static final String DEFAULT_PACKAGING = "jar";
 
     static Pom parse(Path path) throws IOException, ParserConfigurationException, SAXException {
-      Document pomDocument =
-          DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(path.getInputStream());
+      Document pomDocument = null;
+      try (InputStream in = path.getInputStream()) {
+        pomDocument = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(in);
+      }
       Node packagingNode = pomDocument.getElementsByTagName("packaging").item(0);
       String packaging = packagingNode == null ? DEFAULT_PACKAGING : packagingNode.getTextContent();
       MavenCoordinate coordinate = MavenCoordinate.create(
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();
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index 8258dccfc0..01c016905f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -268,12 +268,14 @@ public abstract class FileSystem {
    * @throws IOException if the digest could not be computed for any reason
    */
   protected byte[] getDigest(final Path path, DigestHashFunction hashFunction) throws IOException {
-    return new ByteSource() {
-      @Override
-      public InputStream openStream() throws IOException {
-        return getInputStream(path);
-      }
-    }.hash(hashFunction.getHash()).asBytes();
+    try (InputStream in = getInputStream(path)) {
+      return new ByteSource() {
+        @Override
+        public InputStream openStream() throws IOException {
+          return in;
+        }
+      }.hash(hashFunction.getHash()).asBytes();
+    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
index 999c470c8b..e2ab1d8002 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
@@ -400,7 +400,10 @@ public class FileSystemUtils {
       throw new IOException("error copying file: "
           + "couldn't delete destination: " + e.getMessage());
     }
-    asByteSource(from).copyTo(asByteSink(to));
+    try (InputStream in = from.getInputStream();
+        OutputStream out = to.getOutputStream()) {
+      ByteStreams.copy(in, out);
+    }
     to.setLastModifiedTime(from.getLastModifiedTime()); // Preserve mtime.
     if (!from.isWritable()) {
       to.setWritable(false); // Make file read-only if original was read-only.
@@ -427,7 +430,10 @@ public class FileSystemUtils {
       // Fallback to a copy.
       FileStatus stat = from.stat(Symlinks.NOFOLLOW);
       if (stat.isFile()) {
-        asByteSource(from).copyTo(asByteSink(to));
+        try (InputStream in = from.getInputStream();
+            OutputStream out = to.getOutputStream()) {
+          ByteStreams.copy(in, out);
+        }
         to.setLastModifiedTime(stat.getLastModifiedTime()); // Preserve mtime.
         if (!from.isWritable()) {
           to.setWritable(false); // Make file read-only if original was read-only.
@@ -817,7 +823,14 @@ public class FileSystemUtils {
    * @throws IOException if there was an error
    */
   public static Iterable<String> readLines(Path inputFile, Charset charset) throws IOException {
-    return asByteSource(inputFile).asCharSource(charset).readLines();
+    try (InputStream in = inputFile.getInputStream()) {
+      return new ByteSource() {
+        @Override
+        public InputStream openStream() throws IOException {
+          return in;
+        }
+      }.asCharSource(charset).readLines();
+    }
   }
 
   /**
@@ -826,14 +839,28 @@ public class FileSystemUtils {
    * @throws IOException if there was an error
    */
   public static byte[] readContent(Path inputFile) throws IOException {
-    return asByteSource(inputFile).read();
+    try (InputStream in = inputFile.getInputStream()) {
+      return new ByteSource() {
+        @Override
+        public InputStream openStream() throws IOException {
+          return in;
+        }
+      }.read();
+    }
   }
 
   /**
    * Reads the entire file using the given charset and returns the contents as a string
    */
   public static String readContent(Path inputFile, Charset charset) throws IOException {
-    return asByteSource(inputFile).asCharSource(charset).read();
+    try (InputStream in = inputFile.getInputStream()) {
+      return new ByteSource() {
+        @Override
+        public InputStream openStream() throws IOException {
+          return in;
+        }
+      }.asCharSource(charset).read();
+    }
   }
 
   /**
@@ -843,11 +870,18 @@ public class FileSystemUtils {
    */
   public static byte[] readContentWithLimit(Path inputFile, int limit) throws IOException {
     Preconditions.checkArgument(limit >= 0, "limit needs to be >=0, but it is %s", limit);
-    ByteSource byteSource = asByteSource(inputFile);
-    byte[] buffer = new byte[limit];
-    try (InputStream inputStream = byteSource.openBufferedStream()) {
-      int read = ByteStreams.read(inputStream, buffer, 0, limit);
-      return read == limit ? buffer : Arrays.copyOf(buffer, read);
+    try (InputStream in = inputFile.getInputStream()) {
+      byte[] buffer = new byte[limit];
+      try (InputStream inputStream =
+          new ByteSource() {
+            @Override
+            public InputStream openStream() throws IOException {
+              return in;
+            }
+          }.openBufferedStream()) {
+        int read = ByteStreams.read(inputStream, buffer, 0, limit);
+        return read == limit ? buffer : Arrays.copyOf(buffer, read);
+      }
     }
   }