Bazel server, tools: ensure Readers are closed

Follow-up to commit 59f17d6e0550bf63a0b6ef182e2d63474e058ede.

Use try-with-resources to ensure Reader objects
are closed eagerly.

Eagerly closing Readers avoids hanging on to
file handles until the garbage collector finalizes
the object, 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: 203771262

2 files changed, 17 insertions, 28 deletions

diff --git a/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/OptionFileExpander.java b/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/OptionFileExpander.java
index 66d4a72579..aa396b5973 100644
--- a/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/OptionFileExpander.java
+++ b/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/OptionFileExpander.java
@@ -18,7 +18,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException;
-
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
@@ -26,7 +25,6 @@ import java.io.InputStreamReader;
 import java.io.Reader;
 import java.util.ArrayList;
 import java.util.List;
-
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -82,11 +80,11 @@ final class OptionFileExpander {
    */
   private void expandArgument(String arg, List<String> expanded) throws IOException {
     if (arg.startsWith("@")) {
-      InputStream in = fileSystem.getInputStream(arg.substring(1));
-      try {
+      try (InputStreamReader reader =
+          new InputStreamReader(fileSystem.getInputStream(arg.substring(1)), ISO_8859_1)) {
         // TODO(bazel-team): This code doesn't handle escaped newlines correctly.
         // ShellUtils doesn't support them either.
-        for (String line : readAllLines(new InputStreamReader(in, ISO_8859_1))) {
+        for (String line : readAllLines(reader)) {
           List<String> parsedTokens = new ArrayList<>();
           try {
             ShellUtils.tokenize(parsedTokens, line);
@@ -97,18 +95,6 @@ final class OptionFileExpander {
             expandArgument(token, expanded);
           }
         }
-        InputStream inToClose = in;
-        in = null;
-        inToClose.close();
-      } finally {
-        if (in != null) {
-          try {
-            in.close();
-          } catch (IOException e) {
-            // Ignore the exception. It can only occur if an exception already
-            // happened and in that case, we want to preserve the original one.
-          }
-        }
       }
     } else {
       expanded.add(arg);
diff --git a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java
index bb287637ea..2ecf40e1d0 100644
--- a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java
+++ b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java
@@ -745,17 +745,20 @@ public class ZipCombinerTest {
     byte[] zipCombinerRaw = out.toByteArray();
 
     new ZipTester(zipCombinerRaw).validate();
-    assertZipFilesEquivalent(new ZipReader(zipCombinerFile), new ZipReader(javaFile));
-  }
-
-  void assertZipFilesEquivalent(ZipReader x, ZipReader y) {
-    Collection<ZipFileEntry> xEntries = x.entries();
-    Collection<ZipFileEntry> yEntries = y.entries();
-    assertThat(xEntries).hasSize(yEntries.size());
-    Iterator<ZipFileEntry> xIter = xEntries.iterator();
-    Iterator<ZipFileEntry> yIter = yEntries.iterator();
-    for (int i = 0; i < xEntries.size(); i++) {
-      assertZipEntryEquivalent(xIter.next(), yIter.next());
+    assertZipFilesEquivalent(zipCombinerFile, javaFile);
+  }
+
+  void assertZipFilesEquivalent(File a, File b) throws IOException {
+    try (ZipReader x = new ZipReader(a);
+        ZipReader y = new ZipReader(b)) {
+      Collection<ZipFileEntry> xEntries = x.entries();
+      Collection<ZipFileEntry> yEntries = y.entries();
+      assertThat(xEntries).hasSize(yEntries.size());
+      Iterator<ZipFileEntry> xIter = xEntries.iterator();
+      Iterator<ZipFileEntry> yIter = yEntries.iterator();
+      for (int i = 0; i < xEntries.size(); i++) {
+        assertZipEntryEquivalent(xIter.next(), yIter.next());
+      }
     }
   }