Move the verification of the JVM earlier in the client so that less time passes between the Ping() and Run() calls and make Ping() and Cancel() calls restart the server timeout interval.

This "fixes" a race condition where the client would call Ping(), the server would time out and then the Run() call would fail. Of course, this is not an principled fix because in theory, the timeout can still happen between the two calls, but now there are only simple file system operations and a tiny bit of CPU use between the two so it should be vanishingly unlikely. We use ->Ping() to verify server liveness after we start it up, so in theory, the timeout could strike between those two calls, too...

TESTED=By running "bazel --max_timeout_secs=2 info install_base" 100 times in a test and running the test 200 times. This procedure triggered the bug pretty reliably.

--
MOS_MIGRATED_REVID=126902519

1 files changed, 10 insertions, 0 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 1887d8f826..519a439af5 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
@@ -406,6 +406,12 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma
     }
   }
 
+  private void restartIdleTimeout() {
+    synchronized (runningCommands) {
+      runningCommands.notify();
+    }
+  }
+
   @Override
   public void ping(PingRequest pingRequest, StreamObserver<PingResponse> streamObserver) {
     Preconditions.checkState(serving);
@@ -417,6 +423,8 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma
 
     streamObserver.onNext(response.build());
     streamObserver.onCompleted();
+
+    restartIdleTimeout();
   }
 
   @Override
@@ -437,5 +445,7 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma
 
     streamObserver.onNext(CancelResponse.newBuilder().setCookie(responseCookie).build());
     streamObserver.onCompleted();
+
+    restartIdleTimeout();
   }
 }