Move request cancellation off the dispatching thread

Because we're running our server in a directExecutor the request handling
thread must not block. However in the case of interrupting the command thread
we can block in at least two ways, one not so obvious. The obvious way is
synchronizing on runningCommands. The not obvious way is that interrupting a
thread has some tricky locking involved which can get tangled up with
operations happening in other threads, which may wind up being blocked on the
dispatcher thread being active. More concretely, a thread producing output for
RpcOutputStream may block as a result of the backpressure mechanisms we have in
place. If this winds up blocking in an opportune way, such that it holds some
lock the cancellation thread needs to make progress, we had a deadlock, since
the cancellation thread was preventing output from being consumed.

--
MOS_MIGRATED_REVID=133134743

1 files changed, 14 insertions, 1 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 4e400445bb..1daa6e9d9c 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
@@ -719,13 +719,26 @@ public class GrpcServerImpl extends RPCServer {
         }
 
         @Override
-        public void cancel(CancelRequest request, StreamObserver<CancelResponse> streamObserver) {
+        public void cancel(
+            final CancelRequest request, final StreamObserver<CancelResponse> streamObserver) {
           log.info("Got cancel message for " + request.getCommandId());
           if (!request.getCookie().equals(requestCookie)) {
             streamObserver.onCompleted();
             return;
           }
 
+          // Actually performing the cancellation can result in some blocking which we don't want
+          // to do on the dispatcher thread, instead offload to command pool.
+          commandExecutorPool.execute(new Runnable() {
+            @Override
+            public void run() {
+              doCancel(request, streamObserver);
+            }
+          });
+        }
+
+        private void doCancel(
+            CancelRequest request, StreamObserver<CancelResponse> streamObserver) {
           try (RunningCommand cancelCommand = new RunningCommand()) {
             synchronized (runningCommands) {
               RunningCommand pendingCommand = runningCommands.get(request.getCommandId());