diff --git a/core/src/main/java/io/grpc/internal/ServerCallImpl.java b/core/src/main/java/io/grpc/internal/ServerCallImpl.java index 6f123e76678..f82d87cade0 100644 --- a/core/src/main/java/io/grpc/internal/ServerCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerCallImpl.java @@ -279,7 +279,11 @@ public ServerStreamListenerImpl( new Context.CancellationListener() { @Override public void cancelled(Context context) { - ServerStreamListenerImpl.this.call.cancelled = true; + // If the context has a cancellation cause then something exceptional happened + // and we should also mark the call as cancelled. + if (context.cancellationCause() != null) { + ServerStreamListenerImpl.this.call.cancelled = true; + } } }, MoreExecutors.directExecutor()); @@ -355,6 +359,8 @@ private void closedInternal(Status status) { } finally { // Cancel context after delivering RPC closure notification to allow the application to // clean up and update any state based on whether onComplete or onCancel was called. + // Note that in failure situations JumpToApplicationThreadServerStreamListener has already + // closed the context. In these situations this cancel() call will be a no-op. context.cancel(null); } } diff --git a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java index 5130bc05aa7..ea49b94e8aa 100644 --- a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java @@ -18,6 +18,7 @@ import static com.google.common.base.Charsets.UTF_8; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -378,6 +379,9 @@ public void streamListener_closedOk() { verify(callListener).onComplete(); assertTrue(context.isCancelled()); assertNull(context.cancellationCause()); + // The call considers cancellation to be an exceptional situation so it should + // not be cancelled with an OK status. + assertFalse(call.isCancelled()); } @Test diff --git a/core/src/test/java/io/grpc/internal/ServerImplTest.java b/core/src/test/java/io/grpc/internal/ServerImplTest.java index 2a9dbd5a1fe..0f5c510f97c 100644 --- a/core/src/test/java/io/grpc/internal/ServerImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerImplTest.java @@ -1196,7 +1196,9 @@ public void testStreamClose_clientOkTriggersDelayedCancellation() throws Excepti context, contextCancelled, null); // For close status OK: - // isCancelled is expected to be true after all pending work is done + // The context isCancelled is expected to be true after all pending work is done, + // but for the call it should be false as it gets set cancelled only if the call + // fails with a non-OK status. assertFalse(callReference.get().isCancelled()); assertFalse(context.get().isCancelled()); streamListener.closed(Status.OK); @@ -1204,7 +1206,7 @@ public void testStreamClose_clientOkTriggersDelayedCancellation() throws Excepti assertFalse(context.get().isCancelled()); assertEquals(1, executor.runDueTasks()); - assertTrue(callReference.get().isCancelled()); + assertFalse(callReference.get().isCancelled()); assertTrue(context.get().isCancelled()); assertTrue(contextCancelled.get()); }