Ensure SettableFutures in remote module are all set.

Failed doing so, will cause Bazel hanging forever. This could be one of causes for #11782. Closes #12426. PiperOrigin-RevId: 341554621
bazelbuild · Nov 10, 2020 · c7c7f5f · c7c7f5f
1 parent e86e422
commit c7c7f5f
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 7 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
@@ -37,6 +37,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.flogger.GoogleLogger;
 import com.google.common.hash.HashCode;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -73,6 +74,8 @@
 /** A RemoteActionCache implementation that uses gRPC calls to a remote cache server. */
 @ThreadSafe
 public class GrpcCacheClient implements RemoteCacheClient, MissingDigestsFinder {
+  private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+
   private final CallCredentialsProvider callCredentialsProvider;
   private final ReferenceCountedChannel channel;
   private final RemoteOptions options;
@@ -345,7 +348,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);
                 }
@@ -373,6 +375,9 @@ public void onCompleted() {
                   future.set(null);
                 } catch (IOException e) {
                   future.setException(e);
+                } catch (RuntimeException e) {
+                  logger.atWarning().withCause(e).log("Unexpected exception");
+                  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
@@ -35,6 +35,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
+import com.google.common.flogger.GoogleLogger;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -92,6 +93,7 @@
 /** A cache for storing artifacts (input and output) as well as the output of running an action. */
 @ThreadSafety.ThreadSafe
 public class RemoteCache implements AutoCloseable {
+  private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
   /** See {@link SpawnExecutionContext#lockOutputFiles()}. */
   @FunctionalInterface
@@ -265,7 +267,12 @@ 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) {
+              logger.atWarning().withCause(e).log("Unexpected exception");
+              outerF.setException(e);
+            }
           }
 
           @Override
@@ -471,6 +478,9 @@ public void onSuccess(Void result) {
               outerF.set(null);
             } catch (IOException e) {
               outerF.setException(e);
+            } catch (RuntimeException e) {
+              logger.atWarning().withCause(e).log("Unexpected exception");
+              outerF.setException(e);
             }
           }
 
@@ -482,6 +492,9 @@ public void onFailure(Throwable t) {
               if (t != e) {
                 t.addSuppressed(e);
               }
+            } catch (RuntimeException e) {
+              logger.atWarning().withCause(e).log("Unexpected exception");
+              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,7 @@
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+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;
@@ -113,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/";
@@ -508,9 +510,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;
@@ -606,8 +615,15 @@ public void close() {
                                 // If the error is due to an expired auth token and we can reset
                                 // the input stream, then try again.
                                 if (authTokenExpired(response) && reset(in)) {
-                                  refreshCredentials();
-                                  uploadAfterCredentialRefresh(upload, result);
+                                  try {
+                                    refreshCredentials();
+                                    uploadAfterCredentialRefresh(upload, result);
+                                  } catch (IOException e) {
+                                    result.setException(e);
+                                  } catch (RuntimeException e) {
+                                    logger.atWarning().withCause(e).log("Unexpected exception");
+                                    result.setException(e);
+                                  }
                                 } else {
                                   result.setException(cause);
                                 }