Fix a bunch of issues related to server startup/shutdown:

- Only try to communicate with the server if it passes verification (i.e. the indicated PID exists and its start time is correct)
- Use only one thread to delete files on exit
- Don't set restart reason to NEW_VERSION unless there is an existing server

RELNOTES: None.
PiperOrigin-RevId: 158131470

1 files changed, 34 insertions, 20 deletions

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 676337e8c6..d9689baaa1 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
@@ -54,8 +54,10 @@ import java.io.StringWriter;
 import java.net.InetSocketAddress;
 import java.nio.charset.Charset;
 import java.security.SecureRandom;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.Exchanger;
@@ -528,15 +530,23 @@ public class GrpcServerImpl implements RPCServer {
   private IdleServerTasks idleServerTasks;
   private final int port;
   boolean serving;
+  private List<Path> filesToDeleteAtExit = new ArrayList<Path>();
 
   public GrpcServerImpl(CommandExecutor commandExecutor, Clock clock, int port,
       Path workspace, Path serverDirectory, int maxIdleSeconds) throws IOException {
+    Runtime.getRuntime().addShutdownHook(new Thread() {
+      @Override
+      public void run() {
+        shutdownHook();
+      }
+    });
+
     // server.pid was written in the C++ launcher after fork() but before exec() .
     // The client only accesses the pid file after connecting to the socket
     // which ensures that it gets the correct pid value.
     pidFile = serverDirectory.getRelative("server.pid.txt");
     pidInFile = new String(FileSystemUtils.readContentAsLatin1(pidFile));
-    deleteAtExit(pidFile, /*deleteParent=*/ false);
+    deleteAtExit(pidFile);
 
     this.commandExecutor = commandExecutor;
     this.clock = clock;
@@ -749,34 +759,38 @@ public class GrpcServerImpl implements RPCServer {
   private void writeServerFile(String name, String contents) throws IOException {
     Path file = serverDirectory.getChild(name);
     FileSystemUtils.writeContentAsLatin1(file, contents);
-    deleteAtExit(file, false);
+    deleteAtExit(file);
   }
 
   protected void disableShutdownHooks() {
     runShutdownHooks.set(false);
   }
 
+  private void shutdownHook() {
+    if (!runShutdownHooks.get()) {
+      return;
+    }
+
+    List<Path> files;
+    synchronized (filesToDeleteAtExit) {
+      files = new ArrayList<>(filesToDeleteAtExit);
+    }
+    for (Path path : files) {
+      try {
+        path.delete();
+      } catch (IOException e) {
+        printStack(e);
+      }
+    }
+  }
+
   /**
    * Schedule the specified file for (attempted) deletion at JVM exit.
    */
-  protected static void deleteAtExit(final Path path, final boolean deleteParent) {
-    Runtime.getRuntime().addShutdownHook(new Thread() {
-        @Override
-        public void run() {
-          if (!runShutdownHooks.get()) {
-            return;
-          }
-
-          try {
-            path.delete();
-            if (deleteParent) {
-              path.getParentDirectory().delete();
-            }
-          } catch (IOException e) {
-            printStack(e);
-          }
-        }
-      });
+  protected void deleteAtExit(final Path path) {
+    synchronized (filesToDeleteAtExit) {
+      filesToDeleteAtExit.add(path);
+    }
   }
 
   static void printStack(IOException e) {