Ensure that shutdown commands end the server process before completion

This change ensures that the server process is terminated before the
client process terminates, when evaluating a command that shuts down
the server.

When completing such a command, the server communicates to the client
that the server will terminate itself by setting a termination_expected
bit in the final RunResponse message. The client then waits up to 60s
for the server process to actually terminate. If it does not, then the
client SIGKILLs the server.

Also makes the gRPC server stop accepting new commands before the
shutdown command completes.

Drive-by fix to comments on Search{Un,Null}aryOption.

RELNOTES: Commands that shut down the server (like "shutdown") now ensure that the server process has terminated before the client process terminates.
PiperOrigin-RevId: 161537480

1 files changed, 21 insertions, 9 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 6809b0b7f7..3571b236d2 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -731,9 +731,8 @@ static void StartServerAndConnect(const WorkspaceLayout *workspace_layout,
   // disaster.
   int server_pid = GetServerPid(server_dir);
   if (server_pid > 0) {
-    if (VerifyServerProcess(server_pid, globals->options->output_base,
-                            globals->options->install_base)) {
-      if (KillServerProcess(server_pid)) {
+    if (VerifyServerProcess(server_pid, globals->options->output_base)) {
+      if (KillServerProcess(server_pid, globals->options->output_base)) {
         fprintf(stderr, "Killed non-responsive server process (pid=%d)\n",
                 server_pid);
         SetRestartReasonIfNotSet(SERVER_UNRESPONSIVE);
@@ -1458,8 +1457,7 @@ bool GrpcBlazeServer::Connect() {
     return false;
   }
 
-  if (!VerifyServerProcess(server_pid, globals->options->output_base,
-      globals->options->install_base)) {
+  if (!VerifyServerProcess(server_pid, globals->options->output_base)) {
     return false;
   }
 
@@ -1587,11 +1585,13 @@ void GrpcBlazeServer::KillRunningServer() {
   while (reader->Read(&response)) {
   }
 
-  // Kill the server process for good measure (if we know the server PID)
+  // Wait for the server process to terminate (if we know the server PID).
+  // If it does not terminate itself gracefully within 1m, terminate it.
   if (globals->server_pid > 0 &&
-      VerifyServerProcess(globals->server_pid, globals->options->output_base,
-                          globals->options->install_base)) {
-    KillServerProcess(globals->server_pid);
+      !AwaitServerProcessTermination(globals->server_pid,
+                                     globals->options->output_base,
+                                     kPostShutdownGracePeriodSeconds)) {
+    KillServerProcess(globals->server_pid, globals->options->output_base);
   }
 
   connected_ = false;
@@ -1599,6 +1599,7 @@ void GrpcBlazeServer::KillRunningServer() {
 
 unsigned int GrpcBlazeServer::Communicate() {
   assert(connected_);
+  assert(globals->server_pid > 0);
 
   vector<string> arg_vector;
   string command = globals->option_processor->GetCommand();
@@ -1643,6 +1644,7 @@ unsigned int GrpcBlazeServer::Communicate() {
   int exit_code = -1;
   bool finished = false;
   bool finished_warning_emitted = false;
+  bool termination_expected = false;
 
   while (reader->Read(&response)) {
     if (finished && !finished_warning_emitted) {
@@ -1659,6 +1661,7 @@ unsigned int GrpcBlazeServer::Communicate() {
 
     if (response.finished()) {
       exit_code = response.exit_code();
+      termination_expected = response.termination_expected();
       finished = true;
     }
 
@@ -1694,6 +1697,15 @@ unsigned int GrpcBlazeServer::Communicate() {
     }
   }
 
+  // If the server has shut down, but does not terminate itself within a 1m
+  // grace period, terminate it.
+  if (termination_expected &&
+      !AwaitServerProcessTermination(globals->server_pid,
+                                     globals->options->output_base,
+                                     kPostShutdownGracePeriodSeconds)) {
+    KillServerProcess(globals->server_pid, globals->options->output_base);
+  }
+
   SendAction(CancelThreadAction::JOIN);
   cancel_thread.join();