Introduce a failFast mode to OutputFormatterCallback#close.

--
MOS_MIGRATED_REVID=139508838

7 files changed, 68 insertions, 33 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/concurrent/ExecutorUtil.java b/src/main/java/com/google/devtools/build/lib/concurrent/ExecutorUtil.java
index 1a5a7720df..07ca266aaa 100644
--- a/src/main/java/com/google/devtools/build/lib/concurrent/ExecutorUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/concurrent/ExecutorUtil.java
@@ -31,30 +31,51 @@ public class ExecutorUtil {
   }
 
   /**
-   * Shutdown the executor. If an interrupt occurs, invoke shutdownNow(), but
+   * Shutdown the executor via shutdownNow(). If an interrupt occurs, ignore it (since the tasks
+   * were already interrupted by the initial shutdownNow) and still block on the eventual
+   * termination of the pool.
+   *
+   * @param executor the executor service.
+   * @return true iff interrupted.
+   */
+  public static boolean uninterruptibleShutdownNow(ExecutorService executor) {
+    return shutdownImpl(executor, /*shutdownNowInitially=*/true, /*shutdownNowOnInterrupt=*/false);
+  }
+
+  /**
+   * Shutdown the executor via shutdown(). If an interrupt occurs, invoke shutdownNow(), but
    * still block on the eventual termination of the pool.
    *
    * @param executor the executor service.
    * @return true iff interrupted.
    */
   public static boolean interruptibleShutdown(ExecutorService executor) {
-    return shutdownImpl(executor, /*interruptible=*/true);
+    return shutdownImpl(
+        executor,
+        /*shutdownNowInitially=*/false,
+        /*shutdownNowOnInterrupt=*/true);
   }
 
   /**
-   * Shutdown the executor. If an interrupt occurs, ignore it and still block on the eventual
-   * termination of the pool. This way, all tasks are guaranteed to have completed normally.
+   * Shutdown the executor via shutdown(). If an interrupt occurs, ignore it and still block on the
+   * eventual termination of the pool. This way, all tasks are guaranteed to have completed
+   * normally.
    *
    * @param executor the executor service.
    * @return true iff interrupted.
    */
   public static boolean uninterruptibleShutdown(ExecutorService executor) {
-    return shutdownImpl(executor, /*interruptible=*/false);
+    return shutdownImpl(executor, /*shutdownNowInitially=*/false, /*shutdownNowOnInterrupt=*/false);
   }
 
-  private static boolean shutdownImpl(ExecutorService executor, boolean interruptible) {
+  private static boolean shutdownImpl(
+      ExecutorService executor, boolean shutdownNowInitially, boolean shutdownNowOnInterrupt) {
     Preconditions.checkState(!executor.isShutdown());
-    executor.shutdown();
+    if (shutdownNowInitially) {
+      executor.shutdownNow();
+    } else {
+      executor.shutdown();
+    }
 
     // Common pattern: check for interrupt, but don't return until all threads
     // are finished.
@@ -64,7 +85,7 @@ public class ExecutorUtil {
         executor.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS);
         break;
       } catch (InterruptedException e) {
-        if (interruptible) {
+        if (shutdownNowOnInterrupt) {
           executor.shutdownNow();
         }
         interrupted = true;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
index c16a592797..696eee2230 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
@@ -134,16 +134,18 @@ public abstract class AbstractBlazeQueryEnvironment<T>
       throw new QueryException(expr, e.getMessage());
     }
     IOException ioExn = null;
+    boolean failFast = true;
     try {
       callback.start();
       evalTopLevelInternal(expr, emptySensingCallback);
+      failFast = false;
     } catch (QueryException e) {
       throw new QueryException(e, expr);
     } catch (InterruptedException e) {
       throw e;
     } finally {
       try {
-        callback.close();
+        callback.close(failFast);
       } catch (IOException e) {
         // Only throw this IOException if we weren't about to throw a different exception.
         ioExn = e;
@@ -200,8 +202,8 @@ public abstract class AbstractBlazeQueryEnvironment<T>
     }
 
     @Override
-    public void close() throws InterruptedException, IOException {
-      callback.close();
+    public void close(boolean failFast) throws InterruptedException, IOException {
+      callback.close(failFast);
     }
 
     boolean isEmpty() {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index ad51cd0ccc..705738ce84 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -1173,9 +1173,11 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     }
 
     @Override
-    public void close() throws IOException, InterruptedException {
-      processLastPending();
-      callback.close();
+    public void close(boolean failFast) throws IOException, InterruptedException {
+      if (!failFast) {
+        processLastPending();
+      }
+      callback.close(failFast);
     }
 
     private void processLastPending() throws IOException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/OutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/engine/OutputFormatterCallback.java
index c0d80f8c6c..5d21c874be 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/OutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/OutputFormatterCallback.java
@@ -36,7 +36,7 @@ public abstract class OutputFormatterCallback<T> implements Callback<T> {
    *
    * <p>Will be called even in the case of an error.
    */
-  public void close() throws InterruptedException, IOException {
+  public void close(boolean failFast) throws InterruptedException, IOException {
   }
 
   /**
@@ -72,9 +72,11 @@ public abstract class OutputFormatterCallback<T> implements Callback<T> {
    */
   public static <T> void processAllTargets(OutputFormatterCallback<T> callback,
       Iterable<T> targets) throws IOException, InterruptedException {
+    boolean failFast = true;
     try {
       callback.start();
       callback.process(targets);
+      failFast = false;
     } catch (InterruptedException e) {
       IOException ioException = callback.getIoException();
       if (ioException != null) {
@@ -85,7 +87,7 @@ public abstract class OutputFormatterCallback<T> implements Callback<T> {
       throw new IllegalStateException("This should not happen, as we are not running any query,"
           + " only printing the results:" + e.getMessage(), e);
     } finally {
-      callback.close();
+      callback.close(failFast);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
index f68e851f2f..7b053163cc 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
@@ -244,8 +244,10 @@ public abstract class OutputFormatter implements Serializable {
     }
 
     @Override
-    public void close() throws IOException {
-      flushAndCheckError(printStream);
+    public void close(boolean failFast) throws IOException {
+      if (!failFast) {
+        flushAndCheckError(printStream);
+      }
     }
   }
 
@@ -329,12 +331,14 @@ public abstract class OutputFormatter implements Serializable {
         }
 
         @Override
-        public void close() throws IOException {
-          final String lineTerm = options.getLineTerminator();
-          for (String packageName : packageNames) {
-            printStream.printf("%s%s", packageName, lineTerm);
+        public void close(boolean failFast) throws IOException {
+          if (!failFast) {
+            final String lineTerm = options.getLineTerminator();
+            for (String packageName : packageNames) {
+              printStream.printf("%s%s", packageName, lineTerm);
+            }
           }
-          super.close();
+          super.close(failFast);
         }
       };
     }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
index e291129f62..317888d073 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
@@ -121,8 +121,10 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter {
       }
 
       @Override
-      public void close() throws IOException {
-        queryResult.build().writeTo(out);
+      public void close(boolean failFast) throws IOException {
+        if (!failFast) {
+          queryResult.build().writeTo(out);
+        }
       }
     };
   }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
index 6671e0214f..193ef18cf7 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
@@ -99,14 +99,16 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter {
       }
 
       @Override
-      public void close() throws IOException {
-        try {
-          Transformer transformer = TransformerFactory.newInstance().newTransformer();
-          transformer.setOutputProperty(OutputKeys.INDENT, "yes");
-          transformer.transform(new DOMSource(doc), new StreamResult(out));
-        } catch (TransformerFactoryConfigurationError | TransformerException e) {
-          // This shouldn't be possible: all the configuration is hard-coded.
-          throw new IllegalStateException("XML output failed", e);
+      public void close(boolean failFast) throws IOException {
+        if (!failFast) {
+          try {
+            Transformer transformer = TransformerFactory.newInstance().newTransformer();
+            transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+            transformer.transform(new DOMSource(doc), new StreamResult(out));
+          } catch (TransformerFactoryConfigurationError | TransformerException e) {
+            // This shouldn't be possible: all the configuration is hard-coded.
+            throw new IllegalStateException("XML output failed", e);
+          }
         }
       }
     };