context/core/netty: Add @CheckReturnValue to Context

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 @@
  * </ul>
  */
 /* @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> V call(Callable<V> 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> 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 {}
 }

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<String> PET = Context.key("pet");

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<String> testKey = Context.key("testing");
-    Context.current().withValue(testKey, "testValue").attach();
+    Context context = Context.current().withValue(testKey, "testValue");
+    Context previous = context.attach();
 
     ClientCallImpl<Void, Void> call = new ClientCallImpl<Void, Void>(
         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<Status> statusFuture = SettableFuture.create();
     call.start(new ClientCall.Listener<Void>() {
@@ -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<Void, Void> call = new ClientCallImpl<Void, Void>(
         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);

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);
     }