Move BlazeRuntime/BlazeCommandDispatcher shutdown to where they are created.

RELNOTES: None.
PiperOrigin-RevId: 184692000

3 files changed, 45 insertions, 66 deletions

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 459e5581f1..83c339d5b7 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
@@ -618,8 +618,7 @@ public final class BlazeRuntime {
    * @param requestStrings
    * @return the filtered request to write to the log.
    */
-  @VisibleForTesting
-  static String getRequestLogString(List<String> requestStrings) {
+  public static String getRequestLogString(List<String> requestStrings) {
     StringBuilder buf = new StringBuilder();
     buf.append('[');
     String sep = "";
@@ -848,7 +847,27 @@ public final class BlazeRuntime {
   private static int serverMain(Iterable<BlazeModule> modules, OutErr outErr, String[] args) {
     InterruptSignalHandler sigintHandler = null;
     try {
-      final RPCServer blazeServer = createBlazeRPCServer(modules, Arrays.asList(args));
+      final RPCServer[] rpcServer = new RPCServer[1];
+      Runnable prepareForAbruptShutdown = () -> rpcServer[0].prepareForAbruptShutdown();
+      BlazeRuntime runtime = newRuntime(modules, Arrays.asList(args), prepareForAbruptShutdown);
+      BlazeCommandDispatcher dispatcher = new BlazeCommandDispatcher(runtime);
+      CommandExecutor commandExecutor = new CommandExecutor(dispatcher);
+      BlazeServerStartupOptions startupOptions =
+          runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class);
+      try {
+        // This is necessary so that Bazel kind of works during bootstrapping, at which time the
+        // gRPC server is not compiled in so that we don't need gRPC for bootstrapping.
+        Class<?> factoryClass = Class.forName(
+            "com.google.devtools.build.lib.server.GrpcServerImpl$Factory");
+        RPCServer.Factory factory = (RPCServer.Factory) factoryClass.getConstructor().newInstance();
+        rpcServer[0] = factory.create(commandExecutor, runtime.getClock(),
+            startupOptions.commandPort,
+            runtime.getWorkspace().getWorkspace(),
+            runtime.getServerDirectory(),
+            startupOptions.maxIdleSeconds);
+      } catch (ReflectiveOperationException | IllegalArgumentException e) {
+        throw new AbruptExitException("gRPC server not compiled in", ExitCode.BLAZE_INTERNAL_ERROR);
+      }
 
       // Register the signal handler.
       sigintHandler =
@@ -856,11 +875,13 @@ public final class BlazeRuntime {
             @Override
             public void run() {
               logger.severe("User interrupt");
-              blazeServer.interrupt();
+              rpcServer[0].interrupt();
             }
           };
 
-      blazeServer.serve();
+      rpcServer[0].serve();
+      runtime.shutdown();
+      dispatcher.shutdown();
       return ExitCode.SUCCESS.getNumericExitCode();
     } catch (OptionsParsingException e) {
       outErr.printErr(e.getMessage());
@@ -896,40 +917,6 @@ public final class BlazeRuntime {
   }
 
   /**
-   * Creates and returns a new Blaze RPCServer. Call {@link RPCServer#serve()} to start the server.
-   */
-  @SuppressWarnings("LiteralClassName")  // bootstrap binary does not have gRPC
-  private static RPCServer createBlazeRPCServer(
-      Iterable<BlazeModule> modules, List<String> args)
-      throws IOException, OptionsParsingException, AbruptExitException {
-    final RPCServer[] rpcServer = new RPCServer[1];
-    Runnable prepareForAbruptShutdown = () -> rpcServer[0].prepareForAbruptShutdown();
-
-    BlazeRuntime runtime = newRuntime(modules, args, prepareForAbruptShutdown);
-    BlazeCommandDispatcher dispatcher = new BlazeCommandDispatcher(runtime);
-    CommandExecutor commandExecutor = new CommandExecutor(runtime, dispatcher);
-
-    BlazeServerStartupOptions startupOptions =
-        runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class);
-    try {
-      // This is necessary so that Bazel kind of works during bootstrapping, at which time the
-      // gRPC server is not compiled in so that we don't need gRPC for bootstrapping.
-      Class<?> factoryClass = Class.forName(
-          "com.google.devtools.build.lib.server.GrpcServerImpl$Factory");
-      RPCServer.Factory factory = (RPCServer.Factory) factoryClass.getConstructor().newInstance();
-      rpcServer[0] = factory.create(commandExecutor, runtime.getClock(),
-          startupOptions.commandPort,
-          runtime.getWorkspace().getWorkspace(),
-          runtime.getServerDirectory(),
-          startupOptions.maxIdleSeconds);
-      return rpcServer[0];
-    } catch (ReflectiveOperationException | IllegalArgumentException e) {
-      throw new AbruptExitException("gRPC server not compiled in", ExitCode.BLAZE_INTERNAL_ERROR);
-    }
-
-  }
-
-  /**
    * Parses the command line arguments into a {@link OptionsParser} object.
    *
    *  <p>This function needs to parse the --option_sources option manually so that the real option
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 514fbd1df1..b36cc65d39 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
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.OutErr;
 import java.util.List;
 import java.util.Optional;
-import java.util.logging.Logger;
 
 /**
  * Executes a Blaze command.
@@ -28,13 +27,9 @@ import java.util.logging.Logger;
  * <p>This is the common execution path between the gRPC server and the legacy AF_UNIX server.
  */
 public class CommandExecutor implements ServerCommand {
-  private static final Logger logger = Logger.getLogger(CommandExecutor.class.getName());
-
-  private final BlazeRuntime runtime;
   private final BlazeCommandDispatcher dispatcher;
 
-  CommandExecutor(BlazeRuntime runtime, BlazeCommandDispatcher dispatcher) {
-    this.runtime = runtime;
+  CommandExecutor(BlazeCommandDispatcher dispatcher) {
     this.dispatcher = dispatcher;
   }
 
@@ -48,9 +43,8 @@ public class CommandExecutor implements ServerCommand {
       long firstContactTime,
       Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
       throws InterruptedException {
-    logger.info(BlazeRuntime.getRequestLogString(args));
 
-    BlazeCommandResult result = dispatcher.exec(
+    return dispatcher.exec(
         invocationPolicy,
         args,
         outErr,
@@ -58,10 +52,5 @@ public class CommandExecutor implements ServerCommand {
         clientDescription,
         firstContactTime,
         startupOptionsTaggedWithBazelRc);
-    if (result.shutdown()) {
-      runtime.shutdown();
-      dispatcher.shutdown();
-    }
-    return result;
   }
 }
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 b768c9040b..58ad98edae 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
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode;
 import com.google.devtools.build.lib.runtime.BlazeCommandResult;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.CommandExecutor;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.server.CommandProtos.CancelRequest;
@@ -55,7 +56,7 @@ import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.net.InetSocketAddress;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -110,11 +111,6 @@ import javax.annotation.concurrent.GuardedBy;
 public class GrpcServerImpl implements RPCServer {
   private static final Logger logger = Logger.getLogger(GrpcServerImpl.class.getName());
 
-  // UTF-8 won't do because we want to be able to pass arbitrary binary strings.
-  // Not that the internals of Bazel handle that correctly, but why not make at least this little
-  // part correct?
-  private static final Charset CHARSET = Charset.forName("ISO-8859-1");
-
   private static final long NANOSECONDS_IN_MS = TimeUnit.MILLISECONDS.toNanos(1);
 
   private class RunningCommand implements AutoCloseable {
@@ -809,11 +805,6 @@ public class GrpcServerImpl implements RPCServer {
       return;
     }
 
-    ImmutableList.Builder<String> args = ImmutableList.builder();
-    for (ByteString requestArg : request.getArgList()) {
-      args.add(requestArg.toString(CHARSET));
-    }
-
     String commandId;
     BlazeCommandResult result;
 
@@ -823,8 +814,12 @@ public class GrpcServerImpl implements RPCServer {
     // Convert the startup options record to Java strings, source first.
     ImmutableList.Builder<Pair<String, String>> startupOptions = ImmutableList.builder();
     for (StartupOption option : request.getStartupOptionsList()) {
-      startupOptions.add(
-          new Pair<>(option.getSource().toString(CHARSET), option.getOption().toString(CHARSET)));
+      // UTF-8 won't do because we want to be able to pass arbitrary binary strings.
+      // Not that the internals of Bazel handle that correctly, but why not make at least this
+      // little part correct?
+      startupOptions.add(new Pair<>(
+          option.getSource().toString(StandardCharsets.ISO_8859_1),
+          option.getOption().toString(StandardCharsets.ISO_8859_1)));
     }
 
     try (RunningCommand command = new RunningCommand()) {
@@ -847,11 +842,19 @@ public class GrpcServerImpl implements RPCServer {
           new RpcOutputStream(command.id, responseCookie, StreamType.STDERR, sink));
 
       try {
+        // UTF-8 won't do because we want to be able to pass arbitrary binary strings.
+        // Not that the internals of Bazel handle that correctly, but why not make at least this
+        // little part correct?
+        ImmutableList<String> args = request.getArgList().stream()
+            .map(arg -> arg.toString(StandardCharsets.ISO_8859_1))
+            .collect(ImmutableList.toImmutableList());
+
         InvocationPolicy policy = InvocationPolicyParser.parsePolicy(request.getInvocationPolicy());
+        logger.info(BlazeRuntime.getRequestLogString(args));
         result =
             commandExecutor.exec(
                 policy,
-                args.build(),
+                args,
                 rpcOutErr,
                 request.getBlockForLock() ? LockingMode.WAIT : LockingMode.ERROR_OUT,
                 request.getClientDescription(),