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

7 files changed, 50 insertions, 36 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
index a4907abfac..d22c376d9d 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
@@ -182,10 +182,10 @@ public class SkylarkRepositoryContextTest {
 
   private void testOutputFile(Path path, String content) throws IOException {
     assertThat(path.exists()).isTrue();
-    assertThat(
-            CharStreams.toString(
-                new InputStreamReader(path.getInputStream(), StandardCharsets.UTF_8)))
-        .isEqualTo(content);
+    try (InputStreamReader reader =
+        new InputStreamReader(path.getInputStream(), StandardCharsets.UTF_8)) {
+      assertThat(CharStreams.toString(reader)).isEqualTo(content);
+    }
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
index 29bfeb24fb..4fc5869276 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
@@ -74,6 +74,7 @@ import io.grpc.inprocess.InProcessServerBuilder;
 import io.grpc.stub.StreamObserver;
 import io.grpc.util.MutableHandlerRegistry;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.concurrent.Executors;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -176,10 +177,10 @@ public class GrpcRemoteCacheTest {
     Scratch scratch = new Scratch();
     scratch.file(authTlsOptions.googleCredentials, new JacksonFactory().toString(json));
 
-    CallCredentials creds =
-        GoogleAuthUtils.newCallCredentials(
-            scratch.resolve(authTlsOptions.googleCredentials).getInputStream(),
-            authTlsOptions.googleAuthScopes);
+    CallCredentials creds = null;
+    try (InputStream in = scratch.resolve(authTlsOptions.googleCredentials).getInputStream()) {
+      GoogleAuthUtils.newCallCredentials(in, authTlsOptions.googleAuthScopes);
+    }
     RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
     RemoteRetrier retrier =
         new RemoteRetrier(
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
index 689e50f24d..e5bf9ea8e9 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Collection;
@@ -133,9 +134,9 @@ public final class Scratch {
   }
 
   public String readFile(String pathName) throws IOException {
-    return new String(
-        ByteStreams.toByteArray(resolve(pathName).getInputStream()),
-        DEFAULT_CHARSET);
+    try (InputStream in = resolve(pathName).getInputStream()) {
+      return new String(ByteStreams.toByteArray(in), DEFAULT_CHARSET);
+    }
   }
 
   /** Like {@code scratch.file}, but the lines are added to the end if the file already exists. */
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java b/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
index 5dc43a2567..42f34c30ae 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
@@ -222,16 +222,16 @@ public class NativePathTest {
   @Test
   public void testInputOutputStreams() throws IOException {
     Path path = fs.getPath(aFile.getPath());
-    OutputStream out = path.getOutputStream();
-    for (int i = 0; i < 256; i++) {
-      out.write(i);
+    try (OutputStream out = path.getOutputStream()) {
+      for (int i = 0; i < 256; i++) {
+        out.write(i);
+      }
     }
-    out.close();
-    InputStream in = path.getInputStream();
-    for (int i = 0; i < 256; i++) {
-      assertThat(in.read()).isEqualTo(i);
+    try (InputStream in = path.getInputStream()) {
+      for (int i = 0; i < 256; i++) {
+        assertThat(in.read()).isEqualTo(i);
+      }
+      assertThat(in.read()).isEqualTo(-1);
     }
-    assertThat(in.read()).isEqualTo(-1);
-    in.close();
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/UnionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/UnionFileSystemTest.java
index 3799cfdd46..b34c0c0412 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/UnionFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/UnionFileSystemTest.java
@@ -201,9 +201,9 @@ public class UnionFileSystemTest extends SymlinkAwareFileSystemTest {
     // Create an "/in" directory directly on the output delegate to bypass the
     // UnionFileSystem's mapping.
     assertThat(inDelegate.getPath("/in").createDirectory()).isTrue();
-    OutputStream outStream = inDelegate.getPath("/in/bar.txt").getOutputStream();
-    outStream.write('i');
-    outStream.close();
+    try (OutputStream outStream = inDelegate.getPath("/in/bar.txt").getOutputStream()) {
+      outStream.write('i');
+    }
 
     Path outFoo = unionfs.getPath("/out/foo");
     unionfs.createSymbolicLink(outFoo, PathFragment.create("../in/bar.txt"));
@@ -218,9 +218,10 @@ public class UnionFileSystemTest extends SymlinkAwareFileSystemTest {
 
     Path resolved = unionfs.resolveSymbolicLinks(outFoo);
     assertThat(resolved.getFileSystem()).isSameAs(unionfs);
-    InputStream barInput = resolved.getInputStream();
-    int barChar = barInput.read();
-    barInput.close();
+    int barChar = -1;
+    try (InputStream barInput = resolved.getInputStream()) {
+      barChar = barInput.read();
+    }
     assertThat(barChar).isEqualTo('i');
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
index 30f4064b60..1989e9bb34 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
@@ -155,10 +155,11 @@ public class InMemoryFileSystemTest extends SymlinkAwareFileSystemTest {
           assertThat(file.isWritable()).isFalse();
           assertThat(file.isExecutable()).isFalse();
           assertThat(file.getLastModifiedTime()).isEqualTo(300);
-          BufferedReader reader = new BufferedReader(
-              new InputStreamReader(file.getInputStream(), Charset.defaultCharset()));
-          assertThat(reader.readLine()).isEqualTo(TEST_FILE_DATA);
-          assertThat(reader.readLine()).isNull();
+          try (BufferedReader reader = new BufferedReader(
+              new InputStreamReader(file.getInputStream(), Charset.defaultCharset()))) {
+            assertThat(reader.readLine()).isEqualTo(TEST_FILE_DATA);
+            assertThat(reader.readLine()).isNull();
+          }
 
           Path symlink = base.getRelative("symlink" + i);
           assertThat(symlink.exists()).isTrue();
@@ -239,10 +240,11 @@ public class InMemoryFileSystemTest extends SymlinkAwareFileSystemTest {
           assertThat(file.isExecutable()).isEqualTo(i % 4 == 0);
           assertThat(file.getLastModifiedTime()).isEqualTo(i);
           if (file.isReadable()) {
-            BufferedReader reader = new BufferedReader(
-                new InputStreamReader(file.getInputStream(), Charset.defaultCharset()));
-            assertThat(reader.readLine()).isEqualTo(TEST_FILE_DATA);
-            assertThat(reader.readLine()).isNull();
+            try (BufferedReader reader = new BufferedReader(
+                new InputStreamReader(file.getInputStream(), Charset.defaultCharset()))) {
+              assertThat(reader.readLine()).isEqualTo(TEST_FILE_DATA);
+              assertThat(reader.readLine()).isNull();
+            }
           }
 
           Path symlink = base.getRelative("symlink_" + threadId + "_" + i);
diff --git a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
index 0f0d79ffb6..870fa67e52 100644
--- a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
+++ b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
@@ -20,12 +20,13 @@ import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.common.io.ByteSource;
 import com.google.devtools.build.lib.skylark.util.SkylarkTestCase;
 import com.google.devtools.build.lib.syntax.ParserInputSource;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skydoc.rendering.RuleInfo;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Paths;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -50,7 +51,15 @@ public final class SkydocTest extends SkylarkTestCase {
       @Override
       public ParserInputSource inputSource(String pathString) throws IOException {
         Path path = fileSystem.getPath(pathString);
-        byte[] bytes = FileSystemUtils.asByteSource(path).read();
+        byte[] bytes = null;
+        try (InputStream in = path.getInputStream()) {
+          bytes = new ByteSource() {
+            @Override
+            public InputStream openStream() throws IOException {
+              return in;
+            }
+          }.read();
+        }
 
         return ParserInputSource.create(bytes, path.asFragment());
       }