mirror of https://github.com/grpc/grpc-java.git
Guarantee missing stream promise delivery
In observed cases, whether RST_STREAM or another failure from netty or the server, listeners can fail to be notified when a connection yields a null stream for the selected streamId. This causes hangs in clients, despite deadlines, with no obvious resolution. Tests which relied upon this promise succeeding must now change.
This commit is contained in:
parent
1fc4ab0bb2
commit
a37d3eb349
|
@ -738,14 +738,19 @@ class NettyClientHandler extends AbstractNettyHandler {
|
|||
|
||||
// Attach the client stream to the HTTP/2 stream object as user data.
|
||||
stream.setHttp2Stream(http2Stream);
|
||||
promise.setSuccess();
|
||||
} else {
|
||||
// Otherwise, the stream has been cancelled and Netty is sending a
|
||||
// RST_STREAM frame which causes it to purge pending writes from the
|
||||
// flow-controller and delete the http2Stream. The stream listener has already
|
||||
// been notified of cancellation so there is nothing to do.
|
||||
//
|
||||
// This process has been observed to fail in some circumstances, leaving listeners
|
||||
// unanswered. Ensure that some exception has been delivered consistent with the
|
||||
// implied RST_STREAM result above.
|
||||
Status status = Status.INTERNAL.withDescription("unknown stream for connection");
|
||||
promise.setFailure(status.asRuntimeException());
|
||||
}
|
||||
// Otherwise, the stream has been cancelled and Netty is sending a
|
||||
// RST_STREAM frame which causes it to purge pending writes from the
|
||||
// flow-controller and delete the http2Stream. The stream listener has already
|
||||
// been notified of cancellation so there is nothing to do.
|
||||
|
||||
// Just forward on the success status to the original promise.
|
||||
promise.setSuccess();
|
||||
} else {
|
||||
Throwable cause = future.cause();
|
||||
if (cause instanceof StreamBufferingEncoder.Http2GoAwayException) {
|
||||
|
|
|
@ -268,7 +268,7 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
|
|||
// Cancel the stream.
|
||||
cancelStream(Status.CANCELLED);
|
||||
|
||||
assertTrue(createFuture.isSuccess());
|
||||
assertFalse(createFuture.isSuccess());
|
||||
verify(streamListener).closed(eq(Status.CANCELLED), same(PROCESSED), any(Metadata.class));
|
||||
}
|
||||
|
||||
|
@ -311,7 +311,7 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
|
|||
ChannelFuture cancelFuture = cancelStream(Status.CANCELLED);
|
||||
assertTrue(cancelFuture.isSuccess());
|
||||
assertTrue(createFuture.isDone());
|
||||
assertTrue(createFuture.isSuccess());
|
||||
assertFalse(createFuture.isSuccess());
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
Loading…
Reference in New Issue