Remove ShutdownBlazeServerException in favor of indicating that the server should be shut down in BlazeCommandResult.

RELNOTES: None.
PiperOrigin-RevId: 184678994

9 files changed, 57 insertions, 102 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java
index 22c1e636f5..f1ffeec5e3 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java
@@ -28,20 +28,16 @@ public interface BlazeCommand {
    * line) via {@link OptionsProvider#getResidue()}. The framework parses and makes available
    * exactly the options that the command class specifies via the annotation {@link
    * Command#options()}. The command indicates success / failure via its return value, which becomes
-   * the Unix exit status of the Blaze client process. It may indicate a shutdown request by
-   * throwing {@link BlazeCommandDispatcher.ShutdownBlazeServerException}. In that case, the Blaze
-   * server process (the memory resident portion of Blaze) will shut down and the exit status will
-   * be 0 (in case the shutdown succeeds without error).
+   * the Unix exit status of the Blaze client process. It may indicate that the server needs to be
+   * shut down or that a particular binary needs to be exec()ed on the terminal where Blaze was
+   * invoked from by returning the appropriate {@link BlazeCommandResult}.
    *
    * @param env The environment for the current command invocation
    * @param options A parsed options instance initialized with the values for the options specified
    *     in {@link Command#options()}.
    * @return The Unix exit status for the Blaze client.
-   * @throws BlazeCommandDispatcher.ShutdownBlazeServerException Indicates that the command wants to
-   *     shutdown the Blaze server.
    */
-  BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options)
-      throws BlazeCommandDispatcher.ShutdownBlazeServerException;
+  BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options);
 
   /**
    * Allows the command to provide command-specific option defaults and/or
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 e58d670e1b..1155611945 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
@@ -53,6 +53,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Dispatches to the Blaze commands; that is, given a command line, this
@@ -61,6 +62,7 @@ import java.util.logging.Level;
  * the runtime state (BlazeRuntime) to the commands.
  */
 public class BlazeCommandDispatcher {
+  private static final Logger logger = Logger.getLogger(BlazeCommandDispatcher.class.getName());
 
   /**
    * What to do if the command lock is not available.
@@ -75,27 +77,6 @@ public class BlazeCommandDispatcher {
   private static final ImmutableSet<String> ALL_HELP_OPTIONS =
       ImmutableSet.of("--help", "-help", "-h");
 
-  /**
-   * By throwing this exception, a command indicates that it wants to shutdown
-   * the Blaze server process.
-   */
-  public static class ShutdownBlazeServerException extends Exception {
-    private final ExitCode exitCode;
-
-    public ShutdownBlazeServerException(ExitCode exitCode, Throwable cause) {
-      super(cause);
-      this.exitCode = exitCode;
-    }
-
-    public ShutdownBlazeServerException(ExitCode exitCode) {
-      this.exitCode = exitCode;
-    }
-
-    public ExitCode getExitCode() {
-      return exitCode;
-    }
-  }
-
   private final BlazeRuntime runtime;
   private final Object commandLock;
   private String currentClientDescription = null;
@@ -134,9 +115,9 @@ public class BlazeCommandDispatcher {
   }
 
   /**
-   * Executes a single command. Returns the Unix exit status for the Blaze client process, or throws
-   * {@link ShutdownBlazeServerException} to indicate that a command wants to shutdown the Blaze
-   * server.
+   * Executes a single command. Returns a {@link BlazeCommandResult} to indicate either an exit
+   * code, the desire to shut down the server, or that a given binary should be executed by the
+   * client.
    */
   BlazeCommandResult exec(
       InvocationPolicy invocationPolicy,
@@ -146,7 +127,7 @@ public class BlazeCommandDispatcher {
       String clientDescription,
       long firstContactTime,
       Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
-      throws ShutdownBlazeServerException, InterruptedException {
+      throws InterruptedException {
     OriginalUnstructuredCommandLineEvent originalCommandLine =
         new OriginalUnstructuredCommandLineEvent(args);
     Preconditions.checkNotNull(clientDescription);
@@ -215,7 +196,7 @@ public class BlazeCommandDispatcher {
         outErr.printErrLn("Server shut down " + shutdownReason);
         return BlazeCommandResult.exitCode(ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
       }
-      return execExclusively(
+      BlazeCommandResult result = execExclusively(
           originalCommandLine,
           invocationPolicy,
           args,
@@ -225,9 +206,12 @@ public class BlazeCommandDispatcher {
           command,
           waitTimeInMs,
           startupOptionsTaggedWithBazelRc);
-    } catch (ShutdownBlazeServerException e) {
-      shutdownReason = "explicitly by client " + currentClientDescription;
-      throw e;
+      if (result.shutdown()) {
+        // TODO(lberki): This also handles the case where we catch an uncaught Throwable in
+        // execExclusively() which is not an explicit shutdown.
+        shutdownReason = "explicitly by client " + clientDescription;
+      }
+      return result;
     } finally {
       synchronized (commandLock) {
         currentClientDescription = null;
@@ -245,8 +229,7 @@ public class BlazeCommandDispatcher {
       String commandName,
       BlazeCommand command,
       long waitTimeInMs,
-      Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
-      throws ShutdownBlazeServerException {
+      Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc) {
     // Record the start time for the profiler. Do not put anything before this!
     long execStartTimeNanos = runtime.getClock().nanoTime();
 
@@ -495,15 +478,12 @@ public class BlazeCommandDispatcher {
         result = BlazeCommandResult.exitCode(moduleExitCode);
       }
       return result;
-    } catch (ShutdownBlazeServerException e) {
-      // result is read in the finally block
-      result = BlazeCommandResult.exitCode(e.getExitCode());
-      throw e;
     } catch (Throwable e) {
       e.printStackTrace();
       BugReport.printBug(outErr, e);
       BugReport.sendBugReport(e, args, crashData);
-      throw new ShutdownBlazeServerException(BugReport.getExitCodeForThrowable(e), e);
+      logger.log(Level.SEVERE, "Shutting down due to exception", e);
+      return BlazeCommandResult.shutdown(BugReport.getExitCodeForThrowable(e));
     } finally {
       env.getEventBus().post(new AfterCommandEvent());
       int numericExitCode = result.getExitCode() == null
@@ -531,9 +511,8 @@ public class BlazeCommandDispatcher {
    * long, Optional<List<Pair<String, String>>>)}, but automatically uses the current time.
    */
   @VisibleForTesting
-  public int exec(
-      List<String> args, LockingMode lockingMode, String clientDescription, OutErr originalOutErr)
-      throws ShutdownBlazeServerException, InterruptedException {
+  public BlazeCommandResult exec(List<String> args, String clientDescription, OutErr originalOutErr)
+      throws InterruptedException {
     return exec(
         InvocationPolicy.getDefaultInstance(),
         args,
@@ -541,7 +520,7 @@ public class BlazeCommandDispatcher {
         LockingMode.ERROR_OUT,
         clientDescription,
         runtime.getClock().currentTimeMillis(),
-        Optional.empty() /* startupOptionBundles */).getExitCode().getNumericExitCode();
+        Optional.empty() /* startupOptionBundles */);
   }
 
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
index c1701c6ece..de861a54f9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
@@ -29,25 +29,35 @@ public final class BlazeCommandResult {
   private final ExitCode exitCode;
   @Nullable
   private final ExecRequest execDescription;
+  private final boolean shutdown;
 
-  private BlazeCommandResult(ExitCode exitCode, ExecRequest execDescription) {
+  private BlazeCommandResult(ExitCode exitCode, ExecRequest execDescription, boolean shutdown) {
     this.exitCode = Preconditions.checkNotNull(exitCode);
     this.execDescription = execDescription;
+    this.shutdown = shutdown;
   }
 
   public ExitCode getExitCode() {
     return exitCode;
   }
 
+  public boolean shutdown() {
+    return shutdown;
+  }
+
   @Nullable public ExecRequest getExecRequest() {
     return execDescription;
   }
 
   public static BlazeCommandResult exitCode(ExitCode exitCode) {
-    return new BlazeCommandResult(exitCode, null);
+    return new BlazeCommandResult(exitCode, null, false);
   }
 
+  public static BlazeCommandResult shutdown(ExitCode exitCode) {
+    return new BlazeCommandResult(exitCode, null, true);
+  }
   public static BlazeCommandResult execute(ExecRequest execDescription) {
-    return new BlazeCommandResult(ExitCode.SUCCESS, Preconditions.checkNotNull(execDescription));
+    return new BlazeCommandResult(
+        ExitCode.SUCCESS, Preconditions.checkNotNull(execDescription), false);
   }
 }
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 7319206e80..459e5581f1 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
@@ -830,8 +830,6 @@ public final class BlazeRuntime {
         logger.log(Level.SEVERE, "Exception while executing binary from 'run' command", e);
         return ExitCode.LOCAL_ENVIRONMENTAL_ERROR.getNumericExitCode();
       }
-    } catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
-      return e.getExitCode().getNumericExitCode();
     } catch (InterruptedException e) {
       // This is almost main(), so it's okay to just swallow it. We are exiting soon.
       return ExitCode.INTERRUPTED.getNumericExitCode();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
index 89c3a57911..514fbd1df1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
@@ -18,8 +18,6 @@ import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.In
 import com.google.devtools.build.lib.server.ServerCommand;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.OutErr;
-import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.util.List;
 import java.util.Optional;
 import java.util.logging.Logger;
@@ -32,12 +30,10 @@ import java.util.logging.Logger;
 public class CommandExecutor implements ServerCommand {
   private static final Logger logger = Logger.getLogger(CommandExecutor.class.getName());
 
-  private boolean shutdown;
   private final BlazeRuntime runtime;
   private final BlazeCommandDispatcher dispatcher;
 
   CommandExecutor(BlazeRuntime runtime, BlazeCommandDispatcher dispatcher) {
-    this.shutdown = false;
     this.runtime = runtime;
     this.dispatcher = dispatcher;
   }
@@ -54,33 +50,18 @@ public class CommandExecutor implements ServerCommand {
       throws InterruptedException {
     logger.info(BlazeRuntime.getRequestLogString(args));
 
-    try {
-      return dispatcher.exec(
-          invocationPolicy,
-          args,
-          outErr,
-          lockingMode,
-          clientDescription,
-          firstContactTime,
-          startupOptionsTaggedWithBazelRc);
-    } catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
-      if (e.getCause() != null) {
-        StringWriter message = new StringWriter();
-        message.write("Shutting down due to exception:\n");
-        PrintWriter writer = new PrintWriter(message, true);
-        e.printStackTrace(writer);
-        writer.flush();
-        logger.severe(message.toString());
-      }
-      shutdown = true;
+    BlazeCommandResult result = dispatcher.exec(
+        invocationPolicy,
+        args,
+        outErr,
+        lockingMode,
+        clientDescription,
+        firstContactTime,
+        startupOptionsTaggedWithBazelRc);
+    if (result.shutdown()) {
       runtime.shutdown();
       dispatcher.shutdown();
-      return BlazeCommandResult.exitCode(e.getExitCode());
     }
-  }
-
-  @Override
-  public boolean shutdown() {
-    return shutdown;
+    return result;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
index bf88f451a0..368a5b362b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.buildtool.OutputDirectoryLinksUtils;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.runtime.BlazeCommand;
-import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.ShutdownBlazeServerException;
 import com.google.devtools.build.lib.runtime.BlazeCommandResult;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -128,8 +127,7 @@ public final class CleanCommand implements BlazeCommand {
   private static final Logger logger = Logger.getLogger(CleanCommand.class.getName());
 
   @Override
-  public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options)
-      throws ShutdownBlazeServerException {
+  public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options) {
     Options cleanOptions = options.getOptions(Options.class);
     boolean async = cleanOptions.async;
     env.getEventBus().post(new NoBuildEvent());
@@ -165,8 +163,7 @@ public final class CleanCommand implements BlazeCommand {
           options
               .getOptions(BuildRequestOptions.class)
               .getSymlinkPrefix(env.getRuntime().getProductName());
-      actuallyClean(env, env.getOutputBase(), cleanOptions.expunge, async, symlinkPrefix);
-      return BlazeCommandResult.exitCode(ExitCode.SUCCESS);
+      return actuallyClean(env, env.getOutputBase(), cleanOptions.expunge, async, symlinkPrefix);
     } catch (IOException e) {
       env.getReporter().handle(Event.error(e.getMessage()));
       return BlazeCommandResult.exitCode(ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
@@ -208,9 +205,9 @@ public final class CleanCommand implements BlazeCommand {
         .execute();
   }
 
-  private void actuallyClean(
+  private BlazeCommandResult actuallyClean(
       CommandEnvironment env, Path outputBase, boolean expunge, boolean async, String symlinkPrefix)
-      throws IOException, ShutdownBlazeServerException, CommandException, ExecException,
+      throws IOException, CommandException, ExecException,
           InterruptedException {
     String workspaceDirectory = env.getWorkspace().getBaseName();
     if (env.getOutputService() != null) {
@@ -266,9 +263,10 @@ public final class CleanCommand implements BlazeCommand {
 
     // shutdown on expunge cleans
     if (expunge) {
-      throw new ShutdownBlazeServerException(ExitCode.SUCCESS);
+      return BlazeCommandResult.shutdown(ExitCode.SUCCESS);
     }
     System.gc();
+    return BlazeCommandResult.exitCode(ExitCode.SUCCESS);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ShutdownCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ShutdownCommand.java
index f242c9d7ce..4808b39b24 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ShutdownCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ShutdownCommand.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.runtime.commands;
 
 import com.google.devtools.build.lib.runtime.BlazeCommand;
-import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.ShutdownBlazeServerException;
 import com.google.devtools.build.lib.runtime.BlazeCommandResult;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -57,8 +56,7 @@ public final class ShutdownCommand implements BlazeCommand {
   public void editOptions(OptionsParser optionsParser) {}
 
   @Override
-  public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options)
-      throws ShutdownBlazeServerException {
+  public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options) {
     int limit = options.getOptions(Options.class).heapSizeLimit;
 
     // Iff limit is non-zero, shut down the server if total memory exceeds the
@@ -70,8 +68,9 @@ public final class ShutdownCommand implements BlazeCommand {
 
     if (limit == 0 ||
         Runtime.getRuntime().totalMemory() > limit * 1000L * 1000) {
-      throw new ShutdownBlazeServerException(ExitCode.SUCCESS);
+      return BlazeCommandResult.shutdown(ExitCode.SUCCESS);
     }
+
     return BlazeCommandResult.exitCode(ExitCode.SUCCESS);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
index aa575da2b5..b768c9040b 100644
--- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
@@ -880,8 +880,7 @@ public class GrpcServerImpl implements RPCServer {
     // the cancel request won't find the thread to interrupt)
     Thread.interrupted();
 
-    boolean shutdown = commandExecutor.shutdown();
-    if (shutdown) {
+    if (result.shutdown()) {
       server.shutdown();
     }
 
@@ -889,7 +888,7 @@ public class GrpcServerImpl implements RPCServer {
         .setCookie(responseCookie)
         .setCommandId(commandId)
         .setFinished(true)
-        .setTerminationExpected(shutdown);
+        .setTerminationExpected(result.shutdown());
 
     if (result.getExecRequest() != null) {
       response.setExitCode(0);
diff --git a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
index aed9d73b98..c47e12853d 100644
--- a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
@@ -45,9 +45,4 @@ public interface ServerCommand {
       long firstContactTime,
       Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
       throws InterruptedException;
-
-  /**
-   * Whether the server needs to be shut down.
-   */
-  boolean shutdown();
 }