Assorted changes wrt. gRPC client/server comms:

- Actually make it work again (commit 00cfb7df61b1f3d9fac8ee29d92b315cbfe6d28f broke it, maybe I shouldn't send out changes in a hurry next time)
- Rename --grpc_port to --command_port (it's a bit better name)
- Do not send a kill signal to the server that can't be connected if we only connect to it to verify its presence

--
MOS_MIGRATED_REVID=120418784

7 files changed, 27 insertions, 25 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index efac21ad5a..3187f1d883 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -363,8 +363,9 @@ static vector<string> GetArgumentArray() {
     result.push_back("--batch");
   }
 
-  if (globals->options.grpc_port != -1) {
-    result.push_back("--grpc_port=" + ToString(globals->options.grpc_port));
+  if (globals->options.command_port != -1) {
+    result.push_back(
+        "--command_port=" + ToString(globals->options.command_port));
   }
 
   result.push_back("--install_base=" +
@@ -890,7 +891,9 @@ static bool ConnectToServer(BlazeServer *server, bool start) {
            "can't get server pid from connection");
     }
     return true;
-  } else {
+  }
+
+  if (start) {
     // If we couldn't connect to the server check if there is still a PID file
     // and if so, kill the server that wrote it. This can happen e.g. if the
     // server is in a GC pause and therefore cannot respond to ping requests and
@@ -900,9 +903,7 @@ static bool ConnectToServer(BlazeServer *server, bool start) {
     if (server_pid >= 0) {
       kill(server_pid, SIGKILL);
     }
-  }
 
-  if (start) {
     SetScheduling(globals->options.batch_cpu_scheduling,
                   globals->options.io_nice_level);
 
@@ -1803,11 +1804,11 @@ int main(int argc, const char *argv[]) {
   const string self_path = GetSelfPath();
   ComputeBaseDirectories(self_path);
 
-  blaze_server = globals->options.grpc_port >= 0
+  blaze_server = globals->options.command_port >= 0
       ? static_cast<BlazeServer *>(new GrpcBlazeServer())
       : static_cast<BlazeServer *>(new AfUnixBlazeServer());
 
-  if (globals->options.grpc_port < 0 || globals->options.batch) {
+  if (globals->options.command_port < 0 || globals->options.batch) {
     // The gRPC server can handle concurrent commands just fine. However, we
     // need to be careful not to start two parallel instances in batch mode.
     AcquireLock();
@@ -1841,15 +1842,15 @@ bool GrpcBlazeServer::Connect() {
   std::string server_dir = globals->options.output_base + "/server";
   std::string port;
 
-  if (!ReadFile(server_dir + "/grpc_port", &port)) {
+  if (!ReadFile(server_dir + "/command_port", &port)) {
     return false;
   }
 
-  if (!ReadFile(server_dir + "/request_cookie_", &request_cookie_)) {
+  if (!ReadFile(server_dir + "/request_cookie", &request_cookie_)) {
     return false;
   }
 
-  if (!ReadFile(server_dir + "/response_cookie_", &response_cookie_)) {
+  if (!ReadFile(server_dir + "/response_cookie", &response_cookie_)) {
     return false;
   }
 
@@ -1865,7 +1866,7 @@ bool GrpcBlazeServer::Connect() {
   command_server::PingRequest request;
   command_server::PingResponse response;
   request.set_cookie(request_cookie_);
-  grpc::Status status = client_->Ping(&context, request, &response);
+  grpc::Status status = client->Ping(&context, request, &response);
 
   if (!status.ok()) {
     return false;
diff --git a/src/main/cpp/blaze_startup_options.cc b/src/main/cpp/blaze_startup_options.cc
index 1f7a5409b6..a7111e8f06 100644
--- a/src/main/cpp/blaze_startup_options.cc
+++ b/src/main/cpp/blaze_startup_options.cc
@@ -56,7 +56,7 @@ BlazeStartupOptions::BlazeStartupOptions(const BlazeStartupOptions &rhs)
       watchfs(rhs.watchfs),
       allow_configurable_attributes(rhs.allow_configurable_attributes),
       option_sources(rhs.option_sources),
-      grpc_port(rhs.grpc_port),
+      command_port(rhs.command_port),
       invocation_policy(rhs.invocation_policy),
       host_javabase(rhs.host_javabase) {}
 
diff --git a/src/main/cpp/blaze_startup_options.h b/src/main/cpp/blaze_startup_options.h
index d44cbbee4c..7ffaa836bb 100644
--- a/src/main/cpp/blaze_startup_options.h
+++ b/src/main/cpp/blaze_startup_options.h
@@ -212,7 +212,7 @@ class BlazeStartupOptions {
 
   // Port for gRPC command server. 0 means let the kernel choose, -1 means no
   // gRPC command server.
-  int grpc_port;
+  int command_port;
 
   // Invocation policy proto. May be NULL.
   const char* invocation_policy;
diff --git a/src/main/cpp/blaze_startup_options_common.cc b/src/main/cpp/blaze_startup_options_common.cc
index 54a649931a..a518bd56c0 100644
--- a/src/main/cpp/blaze_startup_options_common.cc
+++ b/src/main/cpp/blaze_startup_options_common.cc
@@ -50,7 +50,7 @@ void BlazeStartupOptions::Init() {
   // 3 hours (but only 5 seconds if used within a test)
   max_idle_secs = testing ? 5 : (3 * 3600);
   oom_more_eagerly_threshold = 100;
-  grpc_port = -1;
+  command_port = -1;
   oom_more_eagerly = false;
   watchfs = false;
   invocation_policy = NULL;
@@ -81,7 +81,7 @@ void BlazeStartupOptions::Copy(
   lhs->batch_cpu_scheduling = rhs.batch_cpu_scheduling;
   lhs->io_nice_level = rhs.io_nice_level;
   lhs->max_idle_secs = rhs.max_idle_secs;
-  lhs->grpc_port = rhs.grpc_port;
+  lhs->command_port = rhs.command_port;
   lhs->oom_more_eagerly = rhs.oom_more_eagerly;
   lhs->watchfs = rhs.watchfs;
   lhs->allow_configurable_attributes = rhs.allow_configurable_attributes;
@@ -233,11 +233,11 @@ blaze_exit_code::ExitCode BlazeStartupOptions::ProcessArg(
     watchfs = false;
     option_sources["watchfs"] = rcfile;
   } else if ((value = GetUnaryOption(
-      arg, next_arg, "--grpc_port")) != NULL) {
-    if (!blaze_util::safe_strto32(value, &grpc_port) ||
-        grpc_port < -1 || grpc_port > 65535) {
+      arg, next_arg, "--command_port")) != NULL) {
+    if (!blaze_util::safe_strto32(value, &command_port) ||
+        command_port < -1 || command_port > 65535) {
       blaze_util::StringPrintf(error,
-          "Invalid argument to --grpc_port: '%s'. "
+          "Invalid argument to --command_port: '%s'. "
           "Must be a valid port number or -1 to disable the gRPC server.\n",
           value);
       return blaze_exit_code::BAD_ARGV;
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 2004ffe64a..ace94597c9 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
@@ -877,7 +877,7 @@ public final class BlazeRuntime {
     CommandExecutor commandExecutor = new CommandExecutor(runtime, dispatcher);
 
 
-    if (startupOptions.grpcPort != -1) {
+    if (startupOptions.commandPort != -1) {
       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.
@@ -885,7 +885,7 @@ public final class BlazeRuntime {
             "com.google.devtools.build.lib.server.GrpcServerImpl$Factory");
         RPCServer.Factory factory = (RPCServer.Factory) factoryClass.newInstance();
         return factory.create(commandExecutor, runtime.getClock(),
-            startupOptions.grpcPort, runtime.getServerDirectory());
+            startupOptions.commandPort, runtime.getServerDirectory());
       } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
         throw new AbruptExitException("gRPC server not compiled in", ExitCode.BLAZE_INTERNAL_ERROR);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index 35bc68e076..b1d1b65545 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -258,9 +258,10 @@ public class BlazeServerStartupOptions extends OptionsBase {
           + "specify --invocation_policy multiple times.")
   public String invocationPolicy;
 
-  @Option(name = "grpc_port",
+  @Option(name = "command_port",
       defaultValue = "-1",
       category = "undocumented",
-      help = "Port to start up the gRPC command server on. If 0, let the kernel choose.")
-  public int grpcPort;
+      help = "Port to start up the gRPC command server on. If 0, let the kernel choose. If -1, "
+          + "use a custom protocol on an AF_UNIX socket.")
+  public int commandPort;
 }
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 8509a25389..477f4b4850 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
@@ -125,7 +125,7 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma
   }
 
   // These paths are all relative to the server directory
-  private static final String PORT_FILE = "grpc_port";
+  private static final String PORT_FILE = "command_port";
   private static final String REQUEST_COOKIE_FILE = "request_cookie";
   private static final String RESPONSE_COOKIE_FILE = "response_cookie";