Skip to content

Commit

Permalink
Ensure NetworkTime never throw exceptions inside onClose
Browse files Browse the repository at this point in the history
Failed doing so will cause gRPC hanging forever. This could be one of causes that leads to #11782.

Closes #12422.

PiperOrigin-RevId: 340995977
  • Loading branch information
coeuvre authored and copybara-github committed Nov 6, 2020
1 parent c282526 commit f9f8ce7
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<NetworkTime> CONTEXT_KEY = Context.key("remote-network-time");

private final Stopwatch wallTime = Stopwatch.createUnstarted();
Expand Down Expand Up @@ -87,9 +90,20 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
new ForwardingClientCallListener.SimpleForwardingClientCallListener<RespT>(
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);
}
},
Expand Down

0 comments on commit f9f8ce7

Please sign in to comment.