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
5a8326f1c7
commit
2a766c7a68
|
@ -738,14 +738,19 @@ class NettyClientHandler extends AbstractNettyHandler {
|
||||||
|
|
||||||
// Attach the client stream to the HTTP/2 stream object as user data.
|
// Attach the client stream to the HTTP/2 stream object as user data.
|
||||||
stream.setHttp2Stream(http2Stream);
|
stream.setHttp2Stream(http2Stream);
|
||||||
}
|
promise.setSuccess();
|
||||||
|
} else {
|
||||||
// Otherwise, the stream has been cancelled and Netty is sending a
|
// Otherwise, the stream has been cancelled and Netty is sending a
|
||||||
// RST_STREAM frame which causes it to purge pending writes from the
|
// RST_STREAM frame which causes it to purge pending writes from the
|
||||||
// flow-controller and delete the http2Stream. The stream listener has already
|
// flow-controller and delete the http2Stream. The stream listener has already
|
||||||
// been notified of cancellation so there is nothing to do.
|
// been notified of cancellation so there is nothing to do.
|
||||||
|
//
|
||||||
// Just forward on the success status to the original promise.
|
// This process has been observed to fail in some circumstances, leaving listeners
|
||||||
promise.setSuccess();
|
// 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());
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
Throwable cause = future.cause();
|
Throwable cause = future.cause();
|
||||||
if (cause instanceof StreamBufferingEncoder.Http2GoAwayException) {
|
if (cause instanceof StreamBufferingEncoder.Http2GoAwayException) {
|
||||||
|
|
|
@ -268,7 +268,7 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
|
||||||
// Cancel the stream.
|
// Cancel the stream.
|
||||||
cancelStream(Status.CANCELLED);
|
cancelStream(Status.CANCELLED);
|
||||||
|
|
||||||
assertTrue(createFuture.isSuccess());
|
assertFalse(createFuture.isSuccess());
|
||||||
verify(streamListener).closed(eq(Status.CANCELLED), same(PROCESSED), any(Metadata.class));
|
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);
|
ChannelFuture cancelFuture = cancelStream(Status.CANCELLED);
|
||||||
assertTrue(cancelFuture.isSuccess());
|
assertTrue(cancelFuture.isSuccess());
|
||||||
assertTrue(createFuture.isDone());
|
assertTrue(createFuture.isDone());
|
||||||
assertTrue(createFuture.isSuccess());
|
assertFalse(createFuture.isSuccess());
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
Loading…
Reference in New Issue