Http2FrameCodec to simulate GOAWAY received when stream IDs are exhausted (#9095)

Motivation:
Http2FrameCodec currently fails the write promise associated with creating a
stream with a Http2NoMoreStreamIdsException. However this means the user code
will have to listen to all write futures in order to catch this scenario which
is the same as receiving a GOAWAY frame. We can also simulate receiving a GOAWAY
frame from our remote peer and that allows users to consolidate graceful close
logic in the GOAWAY processing.

Modifications:
- Http2FrameCodec should simulate a DefaultHttp2GoAwayFrame when trying to
create a stream but the stream IDs have been exhausted.

Result:
Applications can rely upon GOAWAY for graceful close processing instead of also
processing write futures.
This commit is contained in:
Scott Mitchell 2019-04-27 10:55:43 -07:00 committed by GitHub
parent ec62af01c7
commit 2c12f09ec9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 8 deletions

View File

@ -35,8 +35,10 @@ import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory; import io.netty.util.internal.logging.InternalLoggerFactory;
import static io.netty.buffer.ByteBufUtil.writeAscii;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_STREAM_ID; import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_STREAM_ID;
import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid; import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid;
import static io.netty.handler.codec.http2.Http2Error.NO_ERROR;
/** /**
* <p><em>This API is very immature.</em> The Http2Connection-based API is currently preferred over this API. * <p><em>This API is very immature.</em> The Http2Connection-based API is currently preferred over this API.
@ -361,6 +363,12 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
final int streamId = connection.local().incrementAndGetNextStreamId(); final int streamId = connection.local().incrementAndGetNextStreamId();
if (streamId < 0) { if (streamId < 0) {
promise.setFailure(new Http2NoMoreStreamIdsException()); promise.setFailure(new Http2NoMoreStreamIdsException());
// Simulate a GOAWAY being received due to stream exhaustion on this connection. We use the maximum
// valid stream ID for the current peer.
onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(connection.isServer() ? Integer.MAX_VALUE :
Integer.MAX_VALUE - 1, NO_ERROR.code(),
writeAscii(ctx.alloc(), "Stream IDs exhausted on local stream creation")));
return; return;
} }
stream.id = streamId; stream.id = streamId;

View File

@ -55,6 +55,7 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid; import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid;
import static io.netty.handler.codec.http2.Http2Error.NO_ERROR;
import static io.netty.handler.codec.http2.Http2TestUtil.anyChannelPromise; import static io.netty.handler.codec.http2.Http2TestUtil.anyChannelPromise;
import static io.netty.handler.codec.http2.Http2TestUtil.anyHttp2Settings; import static io.netty.handler.codec.http2.Http2TestUtil.anyHttp2Settings;
import static io.netty.handler.codec.http2.Http2TestUtil.assertEqualsAndRelease; import static io.netty.handler.codec.http2.Http2TestUtil.assertEqualsAndRelease;
@ -303,9 +304,9 @@ public class Http2FrameCodecTest {
Http2HeadersFrame actualHeaders = inboundHandler.readInbound(); Http2HeadersFrame actualHeaders = inboundHandler.readInbound();
assertEquals(expectedHeaders.stream(actualHeaders.stream()), actualHeaders); assertEquals(expectedHeaders.stream(actualHeaders.stream()), actualHeaders);
frameInboundWriter.writeInboundRstStream(3, Http2Error.NO_ERROR.code()); frameInboundWriter.writeInboundRstStream(3, NO_ERROR.code());
Http2ResetFrame expectedRst = new DefaultHttp2ResetFrame(Http2Error.NO_ERROR).stream(actualHeaders.stream()); Http2ResetFrame expectedRst = new DefaultHttp2ResetFrame(NO_ERROR).stream(actualHeaders.stream());
Http2ResetFrame actualRst = inboundHandler.readInbound(); Http2ResetFrame actualRst = inboundHandler.readInbound();
assertEquals(expectedRst, actualRst); assertEquals(expectedRst, actualRst);
@ -322,12 +323,12 @@ public class Http2FrameCodecTest {
ByteBuf debugData = bb("debug"); ByteBuf debugData = bb("debug");
ByteBuf expected = debugData.copy(); ByteBuf expected = debugData.copy();
Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(Http2Error.NO_ERROR.code(), debugData); Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), debugData);
goAwayFrame.setExtraStreamIds(2); goAwayFrame.setExtraStreamIds(2);
channel.writeOutbound(goAwayFrame); channel.writeOutbound(goAwayFrame);
verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(7), verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(7),
eq(Http2Error.NO_ERROR.code()), eq(expected), anyChannelPromise()); eq(NO_ERROR.code()), eq(expected), anyChannelPromise());
assertEquals(1, debugData.refCnt()); assertEquals(1, debugData.refCnt());
assertEquals(State.OPEN, stream.state()); assertEquals(State.OPEN, stream.state());
assertTrue(channel.isActive()); assertTrue(channel.isActive());
@ -338,8 +339,8 @@ public class Http2FrameCodecTest {
@Test @Test
public void receiveGoaway() throws Exception { public void receiveGoaway() throws Exception {
ByteBuf debugData = bb("foo"); ByteBuf debugData = bb("foo");
frameInboundWriter.writeInboundGoAway(2, Http2Error.NO_ERROR.code(), debugData); frameInboundWriter.writeInboundGoAway(2, NO_ERROR.code(), debugData);
Http2GoAwayFrame expectedFrame = new DefaultHttp2GoAwayFrame(2, Http2Error.NO_ERROR.code(), bb("foo")); Http2GoAwayFrame expectedFrame = new DefaultHttp2GoAwayFrame(2, NO_ERROR.code(), bb("foo"));
Http2GoAwayFrame actualFrame = inboundHandler.readInbound(); Http2GoAwayFrame actualFrame = inboundHandler.readInbound();
assertEqualsAndRelease(expectedFrame, actualFrame); assertEqualsAndRelease(expectedFrame, actualFrame);
@ -385,13 +386,13 @@ public class Http2FrameCodecTest {
assertEquals(State.OPEN, stream.state()); assertEquals(State.OPEN, stream.state());
ByteBuf debugData = bb("debug"); ByteBuf debugData = bb("debug");
Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(Http2Error.NO_ERROR.code(), debugData.slice()); Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), debugData.slice());
goAwayFrame.setExtraStreamIds(Integer.MAX_VALUE); goAwayFrame.setExtraStreamIds(Integer.MAX_VALUE);
channel.writeOutbound(goAwayFrame); channel.writeOutbound(goAwayFrame);
// When the last stream id computation overflows, the last stream id should just be set to 2^31 - 1. // When the last stream id computation overflows, the last stream id should just be set to 2^31 - 1.
verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(Integer.MAX_VALUE), verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(Integer.MAX_VALUE),
eq(Http2Error.NO_ERROR.code()), eq(debugData), anyChannelPromise()); eq(NO_ERROR.code()), eq(debugData), anyChannelPromise());
assertEquals(1, debugData.refCnt()); assertEquals(1, debugData.refCnt());
assertEquals(State.OPEN, stream.state()); assertEquals(State.OPEN, stream.state());
assertTrue(channel.isActive()); assertTrue(channel.isActive());
@ -675,6 +676,11 @@ public class Http2FrameCodecTest {
ChannelPromise writePromise = channel.newPromise(); ChannelPromise writePromise = channel.newPromise();
channel.writeAndFlush(new DefaultHttp2HeadersFrame(new DefaultHttp2Headers()).stream(stream), writePromise); channel.writeAndFlush(new DefaultHttp2HeadersFrame(new DefaultHttp2Headers()).stream(stream), writePromise);
Http2GoAwayFrame goAwayFrame = inboundHandler.readInbound();
assertNotNull(goAwayFrame);
assertEquals(NO_ERROR.code(), goAwayFrame.errorCode());
assertEquals(Integer.MAX_VALUE, goAwayFrame.lastStreamId());
goAwayFrame.release();
assertThat(writePromise.cause(), instanceOf(Http2NoMoreStreamIdsException.class)); assertThat(writePromise.cause(), instanceOf(Http2NoMoreStreamIdsException.class));
} }