Fix a race condition introduced in unknown commit (that is, the January of 2009!).

If a "blaze clean --expunge" was run concurrently with another command (that was waiting for the lock), it's possible that the clean command deletes the lock file, the new server starts up, then the JVM shutdown hooks delete the PID files from the *new* server.

There is still a slight possibility of a race condition if the lock is deleted then IOException occurs which prevents the BlazeShutdownException from being raised, but I'd rather not introduce another channel from command implementations to RPCServer to close that loophole.

This issue was triggered by commit 5a78166ee4edbd295f5d5fdb94785025285e764b, after which the PID files for the new server are written a bit more early, thus increasing the time window in which the race condition can happen.

--
MOS_MIGRATED_REVID=120805163

1 files changed, 13 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/server/AfUnixServer.java b/src/main/java/com/google/devtools/build/lib/server/AfUnixServer.java
index 558b87569a..f615d04cd7 100644
--- a/src/main/java/com/google/devtools/build/lib/server/AfUnixServer.java
+++ b/src/main/java/com/google/devtools/build/lib/server/AfUnixServer.java
@@ -18,6 +18,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.ByteStreams;
+import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.ShutdownMethod;
 import com.google.devtools.build.lib.server.RPCService.UnknownCommandException;
 import com.google.devtools.build.lib.unix.LocalClientSocket;
 import com.google.devtools.build.lib.unix.LocalServerSocket;
@@ -225,8 +226,16 @@ public final class AfUnixServer extends RPCServer {
               }
             }
             requestIo.shutdown();
-            if (rpcService.isShutdown()) {
-              return;
+            switch (rpcService.getShutdown()) {
+              case NONE:
+                break;
+
+              case CLEAN:
+                return;
+
+              case EXPUNGE:
+                disableShutdownHooks();
+                return;
             }
           }
         } catch (EOFException e) {
@@ -238,7 +247,7 @@ public final class AfUnixServer extends RPCServer {
         }
       }
     } finally {
-      rpcService.shutdown();
+      rpcService.shutdown(ShutdownMethod.CLEAN);
       LOG.info("Logging finished");
     }
   }
@@ -413,7 +422,7 @@ public final class AfUnixServer extends RPCServer {
       LOG.severe("SERVER ERROR: " + trace);
     }
 
-    if (rpcService.isShutdown()) {
+    if (rpcService.getShutdown() != ShutdownMethod.NONE) {
       // In case of shutdown, disable the listening socket *before* we write
       // the last part of the response.  Otherwise, a sufficiently fast client
       // could read the response and exit, and a new client could make a
@@ -533,10 +542,6 @@ public final class AfUnixServer extends RPCServer {
                                         Path workspaceDir,
                                         int maxIdleSeconds)
       throws IOException {
-    if (!serverDirectory.exists()) {
-      serverDirectory.createDirectory();
-    }
-
     // Creates and starts the RPC server.
     RPCService service = new RPCService(appCommand);