Add hook to inform SkyframeExecutor that build is finished.

PiperOrigin-RevId: 194099006

5 files changed, 54 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index ed9024a4d0..6a1e0b9f73 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -77,6 +77,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
@@ -550,10 +551,29 @@ public class BuildTool {
     Preconditions.checkState((crash == null) || !exitCondition.equals(ExitCode.SUCCESS));
     result.setUnhandledThrowable(crash);
     result.setExitCondition(exitCondition);
+    InterruptedException ie = null;
+    try {
+      env.getSkyframeExecutor().notifyCommandComplete();
+    } catch (InterruptedException e) {
+      env.getReporter().handle(Event.error("Build interrupted during command completion"));
+      ie = e;
+    }
     // The stop time has to be captured before we send the BuildCompleteEvent.
     result.setStopTime(runtime.getClock().currentTimeMillis());
     env.getEventBus()
         .post(new BuildCompleteEvent(result, ImmutableList.of(BuildEventId.buildToolLogs())));
+    if (ie != null) {
+      if (exitCondition.equals(ExitCode.SUCCESS)) {
+        result.setExitCondition(ExitCode.INTERRUPTED);
+      } else if (!exitCondition.equals(ExitCode.INTERRUPTED)) {
+        logger.log(
+            Level.WARNING,
+            "Suppressed interrupted exception during stop request because already failing with exit"
+                + " code "
+                + exitCondition,
+            ie);
+      }
+    }
   }
 
   private void reportTargets(AnalysisResult analysisResult) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 4325524c9c..6844412463 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -489,7 +489,7 @@ public class BlazeCommandDispatcher {
       int numericExitCode = result.getExitCode() == null
           ? 0
           : result.getExitCode().getNumericExitCode();
-      runtime.afterCommand(env, numericExitCode);
+      numericExitCode = runtime.afterCommand(env, numericExitCode);
       // Swallow IOException, as we are already in a finally clause
       Flushables.flushQuietly(outErr.getOutputStream());
       Flushables.flushQuietly(outErr.getErrorStream());
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index b7636069d9..064f020e2a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -458,9 +458,12 @@ public final class BlazeRuntime {
     workspace.getSkyframeExecutor().getEventBus().post(new CommandCompleteEvent(exitCode));
   }
 
-  /** Hook method called by the BlazeCommandDispatcher after the dispatch of each command. */
+  /**
+   * Hook method called by the BlazeCommandDispatcher after the dispatch of each command. Returns a
+   * new exit code in case exceptions were encountered during cleanup.
+   */
   @VisibleForTesting
-  public void afterCommand(CommandEnvironment env, int exitCode) {
+  public int afterCommand(CommandEnvironment env, int exitCode) {
     // Remove any filters that the command might have added to the reporter.
     env.getReporter().setOutputFilter(OutputFilter.OUTPUT_EVERYTHING);
 
@@ -479,6 +482,16 @@ public final class BlazeRuntime {
       workspace.getSkyframeExecutor().resetEvaluator();
     }
 
+    // Build-related commands already call this hook in BuildTool#stopRequest, but non-build
+    // commands might also need to notify the SkyframeExecutor. It's called in #stopRequest so that
+    // timing metrics for builds can be more accurate (since this call can be slow).
+    try {
+      workspace.getSkyframeExecutor().notifyCommandComplete();
+    } catch (InterruptedException e) {
+      exitCode = ExitCode.INTERRUPTED.getNumericExitCode();
+      Thread.currentThread().interrupt();
+    }
+
     env.getBlazeWorkspace().clearEventBus();
 
     try {
@@ -490,6 +503,7 @@ public final class BlazeRuntime {
     env.getReporter().clearEventBus();
 
     actionKeyContext.clear();
+    return exitCode;
   }
 
   // Make sure we keep a strong reference to this logger, so that the
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index e0fc14bf5c..293a537b8a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -669,6 +669,15 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     skyframeBuildView.clearLegacyData();
   }
 
+  /**
+   * Notifies the executor that the command is complete. May safely be called multiple times for a
+   * single command, so callers should err on the side of calling it more frequently. Should be
+   * idempotent, so that calls after the first one in the same evaluation should be quick.
+   */
+  public final void notifyCommandComplete() throws InterruptedException {
+    memoizingEvaluator.noteEvaluationsAtSameVersionMayBeFinished();
+  }
+
   protected abstract Differencer evaluatorDiffer();
 
   protected abstract BuildDriver getBuildDriver();
diff --git a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
index 5c7d386555..dac0927ca0 100644
--- a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
@@ -99,6 +99,14 @@ public interface MemoizingEvaluator {
   Map<SkyKey, ? extends NodeEntry> getGraphMap();
 
   /**
+   * Informs the evaluator that a sequence of evaluations at the same version has finished.
+   * Evaluators may make optimizations under the assumption that successive evaluations are all at
+   * the same version. A call of this method tells the evaluator that the next evaluation is not
+   * guaranteed to be at the same version.
+   */
+  default void noteEvaluationsAtSameVersionMayBeFinished() throws InterruptedException {}
+
+  /**
    * Returns the done (without error) values in the graph.
    *
    * <p>The returned map may be a live view of the graph.