Ensure SettableFutures in remote module are all set.

Failed doing so, will cause Bazel hanging forever. This could be one of causes for bazelbuild#11782. Closes bazelbuild#12426. PiperOrigin-RevId: 341554621 # Conflicts: # src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java # src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
coeuvre · Jan 28, 2021 · fb83b98 · fb83b98
1 parent 6cd8bc3
commit fb83b98
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 5 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
@@ -73,6 +73,7 @@
 @ThreadSafe
 public class GrpcCacheClient implements RemoteCacheClient, MissingDigestsFinder {
   private final CallCredentials credentials;
+
   private final ReferenceCountedChannel channel;
   private final RemoteOptions options;
   private final DigestUtil digestUtil;
@@ -326,7 +327,6 @@ public void onNext(ReadResponse readResponse) {
                   data.writeTo(out);
                   offset.addAndGet(data.size());
                 } catch (IOException e) {
-                  future.setException(e);
                   // Cancel the call.
                   throw new RuntimeException(e);
                 }
@@ -355,6 +355,8 @@ public void onCompleted() {
                   future.set(null);
                 } catch (IOException e) {
                   future.setException(e);
+                } catch (RuntimeException e) {
+                  future.setException(e);
                 }
               }
             });

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -263,7 +263,11 @@ public ListenableFuture<byte[]> downloadBlob(Digest digest) {
         new FutureCallback<Void>() {
           @Override
           public void onSuccess(Void aVoid) {
-            outerF.set(bOut.toByteArray());
+            try {
+              outerF.set(bOut.toByteArray());
+            } catch (RuntimeException e) {
+              outerF.setException(e);
+            }
           }
 
           @Override
@@ -453,6 +457,8 @@ public void onSuccess(Void result) {
               outerF.set(null);
             } catch (IOException e) {
               outerF.setException(e);
+            } catch (RuntimeException e) {
+              outerF.setException(e);
             }
           }
 
@@ -464,6 +470,8 @@ public void onFailure(Throwable t) {
               if (t != e) {
                 t.addSuppressed(e);
               }
+            } catch (RuntimeException e) {
+              t.addSuppressed(e);
             } finally {
               outerF.setException(t);
             }

diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD
@@ -23,6 +23,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/remote/util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//third_party:auth",
+        "//third_party:flogger",
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party:netty",

diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
@@ -19,6 +19,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.hash.HashingOutputStream;
+import com.google.common.flogger.GoogleLogger;
+import com.google.common.io.Closeables;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -112,6 +114,7 @@
  * <p>The implementation currently does not support transfer encoding chunked.
  */
 public final class HttpCacheClient implements RemoteCacheClient {
+  private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
   public static final String AC_PREFIX = "ac/";
   public static final String CAS_PREFIX = "cas/";
@@ -512,9 +515,16 @@ public void flush() throws IOException {
                               if (!dataWritten.get() && authTokenExpired(response)) {
                                 // The error is due to an auth token having expired. Let's try
                                 // again.
-                                refreshCredentials();
-                                getAfterCredentialRefresh(downloadCmd, outerF);
-                                return;
+                                try {
+                                  refreshCredentials();
+                                  getAfterCredentialRefresh(downloadCmd, outerF);
+                                  return;
+                                } catch (IOException e) {
+                                  cause.addSuppressed(e);
+                                } catch (RuntimeException e) {
+                                  logger.atWarning().withCause(e).log("Unexpected exception");
+                                  cause.addSuppressed(e);
+                                }
                               } else if (cacheMiss(response.status())) {
                                 outerF.setException(new CacheNotFoundException(digest));
                                 return;