diff --git a/context/src/main/java/io/grpc/Context.java b/context/src/main/java/io/grpc/Context.java index 600d0c25a20..a0ab622f8e5 100644 --- a/context/src/main/java/io/grpc/Context.java +++ b/context/src/main/java/io/grpc/Context.java @@ -16,6 +16,7 @@ package io.grpc; +import io.grpc.Context.CheckReturnValue; import java.io.Closeable; import java.util.ArrayList; import java.util.concurrent.Callable; @@ -93,6 +94,7 @@ * */ /* @DoNotMock("Use ROOT for a non-null Context") // commented out to avoid dependencies */ +@CheckReturnValue public class Context { private static final Logger log = Logger.getLogger(Context.class.getName()); @@ -571,6 +573,7 @@ public void run(Runnable r) { * @param c {@link Callable} to call. * @return result of call. */ + @CanIgnoreReturnValue public V call(Callable c) throws Exception { Context previous = attach(); try { @@ -776,6 +779,7 @@ public boolean isCurrent() { * @return {@code true} if this context cancelled the context and notified listeners, * {@code false} if the context was already cancelled. */ + @CanIgnoreReturnValue public boolean cancel(Throwable cause) { boolean triggeredCancel = false; synchronized (this) { @@ -1008,6 +1012,7 @@ public void cancelled(Context context) { } } + @CanIgnoreReturnValue private static T checkNotNull(T reference, Object errorMessage) { if (reference == null) { throw new NullPointerException(String.valueOf(errorMessage)); @@ -1059,4 +1064,10 @@ private static void validateGeneration(int generation) { new Exception()); } } + + // Not using the standard com.google.errorprone.annotations.CheckReturnValue because that will + // introduce dependencies that some io.grpc.Context API consumers may not want. + @interface CheckReturnValue {} + + @interface CanIgnoreReturnValue {} } diff --git a/context/src/test/java/io/grpc/ContextTest.java b/context/src/test/java/io/grpc/ContextTest.java index 4f8e082ab6e..09ab63ecac1 100644 --- a/context/src/test/java/io/grpc/ContextTest.java +++ b/context/src/test/java/io/grpc/ContextTest.java @@ -60,6 +60,7 @@ * Tests for {@link Context}. */ @RunWith(JUnit4.class) +@SuppressWarnings("CheckReturnValue") // false-positive in test for current ver errorprone plugin public class ContextTest { private static final Context.Key PET = Context.key("pet"); diff --git a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java index f040410deee..d88ff104ecf 100644 --- a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java @@ -137,7 +137,6 @@ public void setUp() { @After public void tearDown() { - Context.ROOT.attach(); verifyZeroInteractions(streamTracerFactory); } @@ -500,7 +499,8 @@ public void prepareHeaders_removeReservedHeaders() { public void callerContextPropagatedToListener() throws Exception { // Attach the context which is recorded when the call is created final Context.Key testKey = Context.key("testing"); - Context.current().withValue(testKey, "testValue").attach(); + Context context = Context.current().withValue(testKey, "testValue"); + Context previous = context.attach(); ClientCallImpl call = new ClientCallImpl( method, @@ -512,10 +512,11 @@ public void callerContextPropagatedToListener() throws Exception { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - Context.ROOT.attach(); + context.detach(previous); // Override the value after creating the call, this should not be seen by callbacks - Context.current().withValue(testKey, "badValue").attach(); + context = Context.current().withValue(testKey, "badValue"); + previous = context.attach(); final AtomicBoolean onHeadersCalled = new AtomicBoolean(); final AtomicBoolean onMessageCalled = new AtomicBoolean(); @@ -555,6 +556,8 @@ private void checkContext() { } }, new Metadata()); + context.detach(previous); + verify(stream).start(listenerArgumentCaptor.capture()); ClientStreamListener listener = listenerArgumentCaptor.getValue(); listener.onReady(); @@ -587,7 +590,7 @@ public void contextCancellationCancelsStream() throws Exception { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - previous.attach(); + cancellableContext.detach(previous); call.start(callListener, new Metadata()); @@ -617,7 +620,7 @@ public void contextAlreadyCancelledNotifiesImmediately() throws Exception { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - previous.attach(); + cancellableContext.detach(previous); final SettableFuture statusFuture = SettableFuture.create(); call.start(new ClientCall.Listener() { @@ -803,9 +806,9 @@ public void expiredDeadlineCancelsStream_CallOptions() { public void expiredDeadlineCancelsStream_Context() { fakeClock.forwardTime(System.nanoTime(), TimeUnit.NANOSECONDS); - Context.current() - .withDeadlineAfter(1, TimeUnit.SECONDS, deadlineCancellationExecutor) - .attach(); + Context context = Context.current() + .withDeadlineAfter(1, TimeUnit.SECONDS, deadlineCancellationExecutor); + Context origContext = context.attach(); ClientCallImpl call = new ClientCallImpl( method, @@ -816,6 +819,8 @@ public void expiredDeadlineCancelsStream_Context() { channelCallTracer, false /* retryEnabled */); + context.detach(origContext); + call.start(callListener, new Metadata()); fakeClock.forwardNanos(TimeUnit.SECONDS.toNanos(1) + 1); diff --git a/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java index fd2120837c8..8b81b6c4b4e 100644 --- a/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java @@ -39,7 +39,6 @@ import com.google.common.util.concurrent.SettableFuture; import io.grpc.Attributes; import io.grpc.CallOptions; -import io.grpc.Context; import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -127,7 +126,6 @@ public void setup() { @After public void teardown() throws Exception { - Context.ROOT.attach(); for (NettyClientTransport transport : transports) { transport.shutdown(Status.UNAVAILABLE); }