Bazel server: ensure OutputStreams are closed

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

Eagerly closing OutputStreams avoids hanging on to
file handles until the garbage collector finalizes
the OutputStream, 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: 203342889

10 files changed, 56 insertions, 28 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/util/DependencySet.java b/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
index e2ce5fb6ce..66f1bd0f25 100644
--- a/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
+++ b/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
@@ -228,15 +228,12 @@ public final class DependencySet {
     Path dotdFile =
         outFile.getRelative(FileSystemUtils.replaceExtension(outFile.asFragment(), suffix));
 
-    PrintStream out = new PrintStream(dotdFile.getOutputStream());
-    try {
+    try (PrintStream out = new PrintStream(dotdFile.getOutputStream())) {
       out.print(outFile.relativeTo(root) + ": ");
       for (Path d : dependencies) {
         out.print(" \\\n  " + d.getPathString());  // should already be root relative
       }
       out.println();
-    } finally {
-      out.close();
     }
   }
 
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 e2ab1d8002..9883030cb6 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
@@ -698,7 +698,14 @@ public class FileSystemUtils {
    */
   public static void writeContent(Path outputFile, Charset charset, String content)
       throws IOException {
-    asByteSink(outputFile).asCharSink(charset).write(content);
+    try (OutputStream out = outputFile.getOutputStream()) {
+      new ByteSink() {
+        @Override
+        public OutputStream openStream() throws IOException {
+          return out;
+        }
+      }.asCharSink(charset).write(content);
+    }
   }
 
   /**
@@ -729,7 +736,14 @@ public class FileSystemUtils {
   public static void writeLinesAs(Path file, Charset charset, Iterable<String> lines)
       throws IOException {
     createDirectoryAndParents(file.getParentDirectory());
-    asByteSink(file).asCharSink(charset).writeLines(lines);
+    try (OutputStream out = file.getOutputStream()) {
+      new ByteSink() {
+        @Override
+        public OutputStream openStream() throws IOException {
+          return out;
+        }
+      }.asCharSink(charset).writeLines(lines);
+    }
   }
 
   /**
@@ -740,7 +754,14 @@ public class FileSystemUtils {
   public static void appendLinesAs(Path file, Charset charset, Iterable<String> lines)
       throws IOException {
     createDirectoryAndParents(file.getParentDirectory());
-    asByteSink(file, true).asCharSink(charset).writeLines(lines);
+    try (OutputStream out = file.getOutputStream(true)) {
+      new ByteSink() {
+        @Override
+        public OutputStream openStream() throws IOException {
+          return out;
+        }
+      }.asCharSink(charset).writeLines(lines);
+    }
   }
 
   /**
@@ -749,7 +770,15 @@ public class FileSystemUtils {
    * @throws IOException if there was an error
    */
   public static void writeContent(Path outputFile, byte[] content) throws IOException {
-    asByteSink(outputFile).write(content);
+    try (OutputStream out = outputFile.getOutputStream()) {
+      new ByteSink() {
+        @Override
+        public OutputStream openStream() throws IOException {
+          return out;
+        }
+      }.write(content);
+      ;
+    }
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
index 9386bcd137..33f03865f6 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.testutil.Suite;
 import com.google.devtools.build.lib.testutil.TestSpec;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.protobuf.ByteString;
@@ -73,8 +74,7 @@ public class SingleBuildFileCacheTest {
           }
         };
     underTest = new SingleBuildFileCache("/", fs);
-    Path file = fs.getPath("/empty");
-    file.getOutputStream().close();
+    FileSystemUtils.createEmptyFile(fs.getPath("/empty"));
   }
 
   @Test
@@ -104,7 +104,7 @@ public class SingleBuildFileCacheTest {
   public void testCache() throws Exception {
     ActionInput empty = ActionInputHelper.fromPath("/empty");
     underTest.getMetadata(empty).getDigest();
-    assert(calls.containsKey("/empty"));
+    assertThat(calls).containsKey("/empty");
     assertThat((int) calls.get("/empty")).isEqualTo(1);
     underTest.getMetadata(empty).getDigest();
     assertThat((int) calls.get("/empty")).isEqualTo(1);
@@ -129,9 +129,9 @@ public class SingleBuildFileCacheTest {
 
     ActionInput input = ActionInputHelper.fromPath("/unreadable");
     Path file = fs.getPath("/unreadable");
-    file.getOutputStream().close();
+    FileSystemUtils.createEmptyFile(file);
     file.chmod(0);
     ByteString actualDigest = ByteString.copyFrom(underTest.getMetadata(input).getDigest());
-    assertThat(expectedDigest).isEqualTo(actualDigest);
+    assertThat(actualDigest).isEqualTo(expectedDigest);
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
index 2f4a8e57bf..c447f67450 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.util.DummyExecutor;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.util.Fingerprint;
+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 com.google.devtools.build.lib.vfs.Symlinks;
@@ -129,7 +130,7 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase {
     CreateIncSymlinkAction action =
         new CreateIncSymlinkAction(NULL_ACTION_OWNER, ImmutableMap.of(a, b), outputDir);
     Path extra = rootDirectory.getRelative("out/extra");
-    extra.getOutputStream().close();
+    FileSystemUtils.createEmptyFile(extra);
     assertThat(extra.exists()).isTrue();
     action.prepare(fileSystem, rootDirectory);
     assertThat(extra.exists()).isFalse();
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
index dd827761be..47540325c7 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
@@ -95,7 +95,7 @@ abstract class BaseSandboxfsProcessTest {
 
       // Create twp mappings: one to be deleted and one to be kept around throughout the test.
       Path keepMeFile = tmpDir.getRelative("one");
-      keepMeFile.getOutputStream().close();
+      FileSystemUtils.createEmptyFile(keepMeFile);
       Path oneFile = tmpDir.getRelative("one");
       FileSystemUtils.writeContent(oneFile, UTF_8, "One test data");
       process.map(
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcessTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcessTest.java
index f9d8c63eb9..06c6b2b234 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcessTest.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import java.io.IOException;
@@ -44,7 +45,7 @@ public class FakeSandboxfsProcessTest extends BaseSandboxfsProcessTest {
 
   @Test
   public void testMount_NotADirectory() throws IOException {
-    tmpDir.getRelative("file").getOutputStream().close();
+    FileSystemUtils.createEmptyFile(tmpDir.getRelative("file"));
     IOException expected = assertThrows(
         IOException.class, () -> mount(tmpDir.getRelative("file")));
     assertThat(expected).hasMessageThat().matches(".*/file.*not a directory");
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index f583465cda..1401bde72a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -1365,11 +1365,8 @@ public class FileFunctionTest {
     return new Runnable() {
       @Override
       public void run() {
-        OutputStream outputStream;
-        try {
-          outputStream = toChange.getOutputStream();
+        try (OutputStream outputStream = toChange.getOutputStream()) {
           outputStream.write(contents);
-          outputStream.close();
         } catch (IOException e) {
           e.printStackTrace();
           fail(e.getMessage());
@@ -1439,9 +1436,9 @@ public class FileFunctionTest {
     Path fileToChange = path(fileStringToChange);
     if (fileToChange.exists()) {
       final byte[] oldContents = FileSystemUtils.readContent(fileToChange);
-      OutputStream outputStream = fileToChange.getOutputStream(/*append=*/ true);
-      outputStream.write(new byte[] {(byte) 42}, 0, 1);
-      outputStream.close();
+      try (OutputStream outputStream = fileToChange.getOutputStream(/*append=*/ true)) {
+        outputStream.write(new byte[] {(byte) 42}, 0, 1);
+      }
       return Pair.of(
           ImmutableList.of(fileStringToChange),
           makeWriteFileContentCallback(fileToChange, oldContents));
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/SearchPathTest.java b/src/test/java/com/google/devtools/build/lib/vfs/SearchPathTest.java
index 6c545ac790..e815a8cc23 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/SearchPathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/SearchPathTest.java
@@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import java.io.OutputStream;
 import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -38,7 +39,9 @@ public class SearchPathTest {
     assertThat(SearchPath.parse(fs, "/:/bin")).isEqualTo(searchPath);
     assertThat(SearchPath.parse(fs, ".:/:/bin")).isEqualTo(searchPath);
 
-    fs.getOutputStream(fs.getPath("/bin/exe")).write(new byte[5]);
+    try (OutputStream out = fs.getOutputStream(fs.getPath("/bin/exe"))) {
+      out.write(new byte[5]);
+    }
 
     assertThat(SearchPath.which(searchPath, "exe")).isNull();
 
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 1989e9bb34..6f2b4e7858 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
@@ -59,9 +59,9 @@ public class InMemoryFileSystemTest extends SymlinkAwareFileSystemTest {
    * Writes the given data to the given file.
    */
   private static void writeToFile(Path path, String data) throws IOException {
-    OutputStream out = path.getOutputStream();
-    out.write(data.getBytes(Charset.defaultCharset()));
-    out.close();
+    try (OutputStream out = path.getOutputStream()) {
+      out.write(data.getBytes(Charset.defaultCharset()));
+    }
   }
 
   /**
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java
index 4010243579..066d93cf7c 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java
@@ -100,7 +100,7 @@ final class ByteStreamServer extends ByteStreamImplBase {
     Path temp = workPath.getRelative("upload").getRelative(UUID.randomUUID().toString());
     try {
       FileSystemUtils.createDirectoryAndParents(temp.getParentDirectory());
-      temp.getOutputStream().close();
+      FileSystemUtils.createEmptyFile(temp);
     } catch (IOException e) {
       logger.log(SEVERE, "Failed to create temporary file for upload", e);
       responseObserver.onError(StatusUtils.internalError(e));