Add OutErr#close, and call that in test strategies.

This fixes "Text file is busy" when trying to execute log files for local tests just
after blaze finishes.

--
MOS_MIGRATED_REVID=86597766

4 files changed, 34 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
index d452afba72..72970a83ae 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
@@ -91,9 +91,10 @@ public class StandaloneTestStrategy extends TestStrategy {
     Executor executor = actionExecutionContext.getExecutor();
 
     ResourceSet resources = null;
+    FileOutErr fileOutErr = null;
     try {
       FileSystemUtils.createDirectoryAndParents(workingDirectory);
-      FileOutErr fileOutErr = new FileOutErr(action.getTestLog().getPath(),
+      fileOutErr = new FileOutErr(action.getTestLog().getPath(),
           action.resolve(actionExecutionContext.getExecutor().getExecRoot()).getTestStderr());
 
       resources = action.getTestProperties().getLocalResourceUsage();
@@ -109,6 +110,13 @@ public class StandaloneTestStrategy extends TestStrategy {
       if (resources != null) {
         ResourceManager.instance().releaseResources(action, resources);
       }
+      try {
+        if (fileOutErr != null) {
+          fileOutErr.close();
+        }
+      } catch (IOException e) {
+        // If the close fails, there is little we can do.
+      }
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index ba24480456..812d1fa89b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -42,6 +42,7 @@ import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.ValueOrException2;
 
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -204,12 +205,13 @@ public class ActionExecutionFunction implements SkyFunction {
       return new ActionExecutionValue(
           fileAndMetadataCache.getOutputData(),
           fileAndMetadataCache.getAdditionalOutputData());
-    } else {
-      ActionExecutionContext actionExecutionContext = null;
+    }
+
+    ActionExecutionContext actionExecutionContext = null;
+    try {
       if (inputArtifactData != null) {
         actionExecutionContext = skyframeActionExecutor.constructActionExecutionContext(
-            fileAndMetadataCache,
-            metadataHandler);
+            fileAndMetadataCache, metadataHandler);
         if (action.discoversInputs()) {
           skyframeActionExecutor.discoverInputs(action, actionExecutionContext);
         }
@@ -224,6 +226,14 @@ public class ActionExecutionFunction implements SkyFunction {
       // result was computed during the call.
       return skyframeActionExecutor.executeAction(action, fileAndMetadataCache, token,
           actionStartTime, actionExecutionContext);
+    } finally {
+      try {
+        if (actionExecutionContext != null) {
+          actionExecutionContext.getFileOutErr().close();
+        }
+      } catch (IOException e) {
+        // Nothing we can do here.
+      }
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index e7f626a1ea..c89631526f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -516,6 +516,7 @@ public final class SkyframeActionExecutor {
    */
   ActionExecutionContext constructActionExecutionContext(final FileAndMetadataCache graphFileCache,
       MetadataHandler metadataHandler) {
+    // TODO(bazel-team): this should be closed explicitly somewhere.
     FileOutErr fileOutErr = actionLogBufferPathGenerator.generate();
     return new ActionExecutionContext(
         executorEngine,
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 c4700eae77..c0988e60cd 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 java.io.Closeable;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
@@ -23,7 +24,7 @@ import java.io.PrintWriter;
  * A pair of output streams to be used for redirecting the output and error
  * streams of a subprocess.
  */
-public class OutErr {
+public class OutErr implements Closeable {
 
   private final OutputStream out;
   private final OutputStream err;
@@ -42,6 +43,14 @@ public class OutErr {
     this.err = err;
   }
 
+  @Override
+  public void close() throws IOException {
+    out.close();
+    if (out != err) {
+      err.close();
+    }
+  }
+
   /**
    * This method redirects {@link System#out} / {@link System#err} into
    * {@code this} object. After calling this method, writing to