commit 55d964efcd1b11c2ecba13786707068983a40e82
author jhorvitz <jhorvitz@google.com> Fri Jul 10 19:31:43 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Jul 10 19:32:49 2020 -0700
tree 0c671cc0cddc92f4b1ba6796c6ae75685d48f500
parent 2a960e93924fc47138b6c422cb3748cb4066784d

Ensure all closeables are always closed.

Found three places where we are not calling close() on other Closeable instances if one throws.

RELNOTES: None.
PiperOrigin-RevId: 320717386

src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index c673a2b..6f982f6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -353,9 +353,13 @@
 
   @Override
   public void close() throws IOException {
-    fileOutErr.close();
-    if (actionFileSystem instanceof Closeable) {
-      ((Closeable) actionFileSystem).close();
+    // Ensure that we close both fileOutErr and actionFileSystem even if one throws.
+    try {
+      fileOutErr.close();
+    } finally {
+      if (actionFileSystem instanceof Closeable) {
+        ((Closeable) actionFileSystem).close();
+      }
     }
   }
 

src/main/java/com/google/devtools/build/lib/util/io/BUILD

diff --git a/src/main/java/com/google/devtools/build/lib/util/io/BUILD b/src/main/java/com/google/devtools/build/lib/util/io/BUILD
index 7c7a55d..804b3c4 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/util/io/BUILD
@@ -41,4 +41,5 @@
 java_library(
     name = "out-err",
     srcs = OUT_ERR_SRCS,
+    deps = ["//third_party:guava"],
 )

src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java

diff --git a/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java b/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
index 233e613..75a4dbc 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
+++ b/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
@@ -85,8 +85,22 @@
 
     @Override
     public void close() throws IOException {
+      // Ensure that we close all sinks even if one throws.
+      IOException firstException = null;
       for (OutputStream sink : sinks) {
-        sink.close();
+        try {
+          sink.close();
+        } catch (IOException e) {
+          if (firstException == null) {
+            firstException = e;
+          } else {
+            firstException.addSuppressed(e);
+          }
+        }
+      }
+
+      if (firstException != null) {
+        throw firstException;
       }
     }
 

src/main/java/com/google/devtools/build/lib/util/io/OutErr.java

diff --git a/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java b/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
index b4c12b9..08942f4 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
+++ b/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.util.io;
 
+import com.google.common.base.Preconditions;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -38,15 +39,19 @@
   }
 
   protected OutErr(OutputStream out, OutputStream err) {
-    this.out = out;
-    this.err = err;
+    this.out = Preconditions.checkNotNull(out);
+    this.err = Preconditions.checkNotNull(err);
   }
 
   @Override
   public void close() throws IOException {
-    out.close();
-    if (out != err) {
-      err.close();
+    // Ensure that we close both out and err even if one throws.
+    try {
+      out.close();
+    } finally {
+      if (out != err) {
+        err.close();
+      }
     }
   }