Bazel client: delete and don't use the server.pid

Upon startup the Bazel client checks if there's
already a running server process and if so then
connects to it.

We achieve this by checking if there's a symlink
in the server directory called served.pid,
pointing to /proc/<server_pid>. If so, we read the
symlink's target and extract the PID; otherwise we
check if there's a file in the server's directory
(server.pid.txt) that contains the PID and read it
from there.

Since the PID file is always there, we don't need
the symlink, plus on Windows we don't support
symlinks anyway, which is the real motivation for
this change.

Just ignoring the PID symlink is not enough, we
need to actively delete it so that switching
between Bazel versions (one that writes a PID
symlink and one that doesn't) won't result in
having a symlink and PID file with different
PIDs and clients trying to kill the wrong server
process / not killing one that they should.

See https://github.com/bazelbuild/bazel/issues/2107

--
MOS_MIGRATED_REVID=140117287

3 files changed, 15 insertions, 20 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 819d3e0ca5..f8422f15fd 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -693,23 +693,11 @@ static void WriteFileToStderrOrDie(const char *file_name) {
 static int GetServerPid(const string &server_dir) {
   // Note: there is no race here on startup since the server creates
   // the pid file strictly before it binds the socket.
-  char buf[33];
-
-  // The server writes a file, but we need to handle old servers that still
-  // write a symlink.
-  int len;
   string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile);
-  string pid_symlink = blaze_util::JoinPath(server_dir, kServerPidSymlink);
-  len = readlink(pid_symlink.c_str(), buf, sizeof(buf) - 1);
   string bufstr;
-  if (len > 0) {
-    bufstr = string(buf, len);
-  } else if (!blaze::ReadFile(pid_file, &bufstr, 32)) {
-    return -1;
-  }
-
   int result;
-  if (!blaze_util::safe_strto32(bufstr, &result)) {
+  if (!blaze::ReadFile(pid_file, &bufstr, 32) ||
+      !blaze_util::safe_strto32(bufstr, &result)) {
     return -1;
   }
 
@@ -727,6 +715,15 @@ static void StartServerAndConnect(BlazeServer *server) {
          "server directory '%s' could not be created", server_dir.c_str());
   }
 
+  // TODO(laszlocsomor) 2016-11-21: remove `pid_symlink` and the `remove` call
+  // after 2017-05-01 (~half a year from writing this comment). By that time old
+  // Bazel clients that used to write PID symlinks will probably no longer be in
+  // use.
+  // Until then, defensively delete old PID symlinks that older clients may have
+  // left behind.
+  string pid_symlink = blaze_util::JoinPath(server_dir, kServerPidSymlink);
+  remove(pid_symlink.c_str());
+
   // 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
diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h
index 0f7b71a596..e294a42205 100644
--- a/src/main/cpp/blaze_util.h
+++ b/src/main/cpp/blaze_util.h
@@ -28,6 +28,10 @@
 namespace blaze {
 
 extern const char kServerPidFile[];
+
+// TODO(laszlocsomor) 2016-11-21: remove kServerPidSymlink after 2017-05-01
+// (~half a year from writing this comment). By that time old Bazel clients that
+// used to write PID symlinks will probably no longer be in use.
 extern const char kServerPidSymlink[];
 
 std::string GetUserName();
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index 55cb59b563..b5d8caf47e 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -160,7 +160,6 @@ void ExecuteDaemon(const string& exe,
   Daemonize(daemon_output);
   string pid_string = GetProcessIdAsString();
   string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile);
-  string pid_symlink_file = blaze_util::JoinPath(server_dir, kServerPidSymlink);
 
   if (!WriteFile(pid_string, pid_file)) {
     // The exit code does not matter because we are already in the daemonized
@@ -168,11 +167,6 @@ void ExecuteDaemon(const string& exe,
     pdie(0, "Cannot write PID file");
   }
 
-  UnlinkPath(pid_symlink_file.c_str());
-  if (symlink(pid_string.c_str(), pid_symlink_file.c_str()) < 0) {
-    pdie(0, "Cannot write PID symlink");
-  }
-
   WriteSystemSpecificProcessIdentifier(server_dir);
 
   ExecuteProgram(exe, args_vector);