From f9f8ce77398470c2f0f22cfd704e15ec3ab024bd Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Thu, 5 Nov 2020 23:26:27 -0800 Subject: [PATCH] Ensure NetworkTime never throw exceptions inside onClose Failed doing so will cause gRPC hanging forever. This could be one of causes that leads to #11782. Closes #12422. PiperOrigin-RevId: 340995977 --- .../google/devtools/build/lib/remote/util/BUILD | 1 + .../build/lib/remote/util/NetworkTime.java | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD index 2b88772002e3d5..2ed7f93bb5a9e0 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD @@ -25,6 +25,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/protobuf:failure_details_java_proto", + "//third_party:flogger", "//third_party:guava", "//third_party:jsr305", "//third_party/grpc:grpc-jar", diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java b/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java index 1f51559d87f377..bdb440a46d4b48 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java @@ -15,6 +15,7 @@ import build.bazel.remote.execution.v2.ExecutionGrpc; import com.google.common.base.Stopwatch; +import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.concurrent.ThreadSafety; import io.grpc.CallOptions; import io.grpc.Channel; @@ -31,6 +32,8 @@ /** Reentrant wall clock stopwatch and grpc interceptor for network waits. */ @ThreadSafety.ThreadSafe public class NetworkTime { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + public static final Context.Key CONTEXT_KEY = Context.key("remote-network-time"); private final Stopwatch wallTime = Stopwatch.createUnstarted(); @@ -87,9 +90,20 @@ public void start(Listener responseListener, Metadata headers) { new ForwardingClientCallListener.SimpleForwardingClientCallListener( responseListener) { + /** + * This method must not throw any exceptions. Doing so will cause the wrapped call to + * silently hang indefinitely: https://github.com/grpc/grpc-java/pull/6107 + */ @Override public void onClose(Status status, Metadata trailers) { - networkTime.stop(); + // There is a risk that networkTime.stop() would throw a IllegalStateException: if + // networkTime.outstanding is overflowed, wallTime.stop() will be called even it's + // already stopped. + try { + networkTime.stop(); + } catch (RuntimeException e) { + logger.atWarning().withCause(e).log("Failed to stop networkTime"); + } super.onClose(status, trailers); } },