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
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 66d4a72..aa396b5 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 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.Reader;
import java.util.ArrayList;
import java.util.List;
-
import javax.annotation.concurrent.Immutable;
/**
@@ -82,11 +80,11 @@
*/
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 @@
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 bb28763..2ecf40e 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 @@
byte[] zipCombinerRaw = out.toByteArray();
new ZipTester(zipCombinerRaw).validate();
- assertZipFilesEquivalent(new ZipReader(zipCombinerFile), new ZipReader(javaFile));
+ assertZipFilesEquivalent(zipCombinerFile, 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());
+ 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());
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/docgen/SourceFileReader.java b/src/main/java/com/google/devtools/build/docgen/SourceFileReader.java
index 17c641e..a6e3e2c 100644
--- a/src/main/java/com/google/devtools/build/docgen/SourceFileReader.java
+++ b/src/main/java/com/google/devtools/build/docgen/SourceFileReader.java
@@ -20,10 +20,11 @@
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import java.io.BufferedReader;
import java.io.File;
-import java.io.FileReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedList;
@@ -296,19 +297,23 @@
return line;
}
+ private static BufferedReader createReader(String filePath) throws IOException {
+ File file = new File(filePath);
+ if (file.exists()) {
+ return Files.newBufferedReader(file.toPath(), StandardCharsets.UTF_8);
+ } else {
+ InputStream is = SourceFileReader.class.getResourceAsStream(filePath);
+ if (is != null) {
+ return new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8));
+ } else {
+ return null;
+ }
+ }
+ }
+
public static void readTextFile(String filePath, ReadAction action)
throws BuildEncyclopediaDocException, IOException {
- BufferedReader br = null;
- try {
- File file = new File(filePath);
- if (file.exists()) {
- br = new BufferedReader(new FileReader(file));
- } else {
- InputStream is = SourceFileReader.class.getResourceAsStream(filePath);
- if (is != null) {
- br = new BufferedReader(new InputStreamReader(is));
- }
- }
+ try (BufferedReader br = createReader(filePath)) {
if (br != null) {
String line = null;
while ((line = br.readLine()) != null) {
@@ -317,10 +322,6 @@
} else {
System.out.println("Couldn't find file or resource: " + filePath);
}
- } finally {
- if (br != null) {
- br.close();
- }
}
}
}
diff --git a/src/test/java/com/google/devtools/build/android/AarGeneratorActionTest.java b/src/test/java/com/google/devtools/build/android/AarGeneratorActionTest.java
index ae40dd3..c5f5323 100644
--- a/src/test/java/com/google/devtools/build/android/AarGeneratorActionTest.java
+++ b/src/test/java/com/google/devtools/build/android/AarGeneratorActionTest.java
@@ -730,14 +730,16 @@
aarData.proguardSpecs);
Set<String> zipEntries = getZipEntries(aar);
assertThat(zipEntries).contains("proguard.txt");
- ZipReader aarReader = new ZipReader(aar.toFile());
- List<String> proguardTxtContents =
- new BufferedReader(
- new InputStreamReader(
- aarReader.getInputStream(aarReader.getEntry("proguard.txt")),
- StandardCharsets.UTF_8))
- .lines()
- .collect(Collectors.toList());
+ List<String> proguardTxtContents = null;
+ try (ZipReader aarReader = new ZipReader(aar.toFile())) {
+ proguardTxtContents =
+ new BufferedReader(
+ new InputStreamReader(
+ aarReader.getInputStream(aarReader.getEntry("proguard.txt")),
+ StandardCharsets.UTF_8))
+ .lines()
+ .collect(Collectors.toList());
+ }
assertThat(proguardTxtContents).containsExactly("foo", "bar", "baz").inOrder();
}
}
diff --git a/src/test/java/com/google/devtools/build/android/ziputils/SplitZipTest.java b/src/test/java/com/google/devtools/build/android/ziputils/SplitZipTest.java
index 1794252..15f1461 100644
--- a/src/test/java/com/google/devtools/build/android/ziputils/SplitZipTest.java
+++ b/src/test/java/com/google/devtools/build/android/ziputils/SplitZipTest.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableSet;
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.io.InputStream;
import java.util.Arrays;
import java.util.Date;
import org.junit.Before;
@@ -423,15 +424,17 @@
String classFileList = "pkg1/test1.class\npkg2/test2.class\n";
fileSystem.addFile("main_dex_list.txt", classFileList);
- new SplitZip()
- .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard1.jar", false),
- "out/shard1.jar"))
- .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard2.jar", false),
- "out/shard2.jar"))
- .setMainClassListFile(fileSystem.getInputStream("main_dex_list.txt"))
- .addInput(new ZipIn(fileSystem.getInputChannel("input.jar"), "input.jar"))
- .run()
- .close();
+ try (InputStream mainDex = fileSystem.getInputStream("main_dex_list.txt")) {
+ new SplitZip()
+ .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard1.jar", false),
+ "out/shard1.jar"))
+ .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard2.jar", false),
+ "out/shard2.jar"))
+ .setMainClassListStreamForTesting(mainDex)
+ .addInput(new ZipIn(fileSystem.getInputChannel("input.jar"), "input.jar"))
+ .run()
+ .close();
+ }
new ZipFileBuilder()
.add("pkg1/test1.class", "hello world")
@@ -531,20 +534,22 @@
String classFileList = "pkg1/test1.class\npkg2/test2.class\n";
fileSystem.addFile("main_dex_list.txt", classFileList);
- new SplitZip()
- .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard1.jar", false),
- "out/shard1.jar"))
- .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard2.jar", false),
- "out/shard2.jar"))
- .setVerbose(true)
- .setMainClassListFile(fileSystem.getInputStream("main_dex_list.txt"))
- .addInput(new ZipIn(fileSystem.getInputChannel("input.jar"), "input.jar"))
- .setInputFilter(
- Predicates.in(
- ImmutableSet.of("pkg1/test1.class", "pkg2/test1.class", "pkg3/test1.class")))
- .setSplitDexedClasses(true)
- .run()
- .close();
+ try (InputStream mainDex = fileSystem.getInputStream("main_dex_list.txt")) {
+ new SplitZip()
+ .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard1.jar", false),
+ "out/shard1.jar"))
+ .addOutput(new ZipOut(fileSystem.getOutputChannel("out/shard2.jar", false),
+ "out/shard2.jar"))
+ .setVerbose(true)
+ .setMainClassListStreamForTesting(mainDex)
+ .addInput(new ZipIn(fileSystem.getInputChannel("input.jar"), "input.jar"))
+ .setInputFilter(
+ Predicates.in(
+ ImmutableSet.of("pkg1/test1.class", "pkg2/test1.class", "pkg3/test1.class")))
+ .setSplitDexedClasses(true)
+ .run()
+ .close();
+ }
// 1st shard contains only main dex list classes also in the filter
new ZipFileBuilder()
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HashInputStreamTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HashInputStreamTest.java
index c1e14df..3c97cf3 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HashInputStreamTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HashInputStreamTest.java
@@ -39,29 +39,30 @@
@Test
public void validChecksum_readsOk() throws Exception {
- assertThat(
- CharStreams.toString(
- new InputStreamReader(
- new HashInputStream(
- new ByteArrayInputStream("hello".getBytes(UTF_8)),
- Hashing.sha1(),
- HashCode.fromString("aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d")),
- UTF_8)))
- .isEqualTo("hello");
+ try (InputStreamReader reader =
+ new InputStreamReader(
+ new HashInputStream(
+ new ByteArrayInputStream("hello".getBytes(UTF_8)),
+ Hashing.sha1(),
+ HashCode.fromString("aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d")),
+ UTF_8)) {
+ assertThat(CharStreams.toString(reader)).isEqualTo("hello");
+ }
}
@Test
public void badChecksum_throwsIOException() throws Exception {
thrown.expect(IOException.class);
thrown.expectMessage("Checksum");
- assertThat(
- CharStreams.toString(
- new InputStreamReader(
- new HashInputStream(
- new ByteArrayInputStream("hello".getBytes(UTF_8)),
- Hashing.sha1(),
- HashCode.fromString("0000000000000000000000000000000000000000")),
- UTF_8)))
- .isNull(); // Only here to make @CheckReturnValue happy.
+ try (InputStreamReader reader =
+ new InputStreamReader(
+ new HashInputStream(
+ new ByteArrayInputStream("hello".getBytes(UTF_8)),
+ Hashing.sha1(),
+ HashCode.fromString("0000000000000000000000000000000000000000")),
+ UTF_8)) {
+ assertThat(CharStreams.toString(reader))
+ .isNull(); // Only here to make @CheckReturnValue happy.
+ }
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
index 656f87c..83ce626 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
@@ -29,7 +29,6 @@
import com.google.protobuf.util.JsonFormat;
import java.io.File;
import java.io.FileInputStream;
-import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import org.junit.After;
@@ -82,8 +81,7 @@
transport.sendBuildEvent(buildEvent, artifactGroupNamer);
transport.close().get();
- try (InputStream in = new FileInputStream(output)) {
- Reader reader = new InputStreamReader(in);
+ try (Reader reader = new InputStreamReader(new FileInputStream(output))) {
JsonFormat.Parser parser = JsonFormat.parser();
BuildEventStreamProtos.BuildEvent.Builder builder =
BuildEventStreamProtos.BuildEvent.newBuilder();
diff --git a/src/tools/android/java/com/google/devtools/build/android/incrementaldeployment/StubApplication.java b/src/tools/android/java/com/google/devtools/build/android/incrementaldeployment/StubApplication.java
index 0b91d03..5cf15fa 100644
--- a/src/tools/android/java/com/google/devtools/build/android/incrementaldeployment/StubApplication.java
+++ b/src/tools/android/java/com/google/devtools/build/android/incrementaldeployment/StubApplication.java
@@ -434,8 +434,7 @@
private static Map<String, String> parseManifest(File file) throws IOException {
Map<String, String> result = new HashMap<>();
- BufferedReader reader = new BufferedReader(new FileReader(file));
- try {
+ try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
while (true) {
String line = reader.readLine();
if (line == null) {
@@ -445,8 +444,6 @@
String[] items = line.split(" ");
result.put(items[0], items[1]);
}
- } finally {
- reader.close();
}
return result;
diff --git a/src/tools/android/java/com/google/devtools/build/android/ziputils/SplitZip.java b/src/tools/android/java/com/google/devtools/build/android/ziputils/SplitZip.java
index 7a1f81a..25a32a0 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ziputils/SplitZip.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ziputils/SplitZip.java
@@ -118,7 +118,7 @@
}
// Package private for testing with mock file
- SplitZip setMainClassListFile(InputStream clInputStream) {
+ SplitZip setMainClassListStreamForTesting(InputStream clInputStream) {
filterInputStream = clInputStream;
return this;
}
@@ -446,21 +446,16 @@
*/
private Set<String> readPaths(String fileName) throws IOException {
Set<String> paths = new HashSet<>();
- BufferedReader reader = null;
- try {
- if (filterInputStream == null) {
- filterInputStream = new FileInputStream(fileName);
- }
- reader = new BufferedReader(new InputStreamReader(filterInputStream, UTF_8));
+ if (filterInputStream == null) {
+ filterInputStream = new FileInputStream(fileName);
+ }
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(filterInputStream, UTF_8))) {
String line;
while (null != (line = reader.readLine())) {
paths.add(fixPath(line));
}
return paths;
- } finally {
- if (reader != null) {
- reader.close();
- }
}
}
