andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

HTTP/2 avoid closing connection when writing GOAWAY (#9227)

Browse Source 
Motivation:
b4e3c12b8e8e984ba65330dd6dc34a4b3d07a25a introduced code to avoid coupling
close() to graceful close. It also added some code which attempted to infer when
a graceful close was being done in writing of a GOAWAY to preserve the
"connection is closed when all streams are closed behavior" for the child
channel API. However the implementation was too overzealous and may preemptively
close the connection if there are not currently any open streams (and close if
there are any frames which create streams in flight).

Modifications:
- Decouple writing a GOAWAY from trying to infer if a graceful close is being
  done and closing the connection. Even if we could enhance this logic (e.g.
wait to close until the second GOAWAY with no error) it is possible the user
doesn't want the connection to be closed yet. We can add a means for the codec
to orchestrate the graceful close in the future (e.g. write some special "close
the connection when all streams are closed") but for now we can just let the
application handle this.

Result:
Fixes https://github.com/netty/netty/issues/9207
This commit is contained in:
Scott Mitchell 2019-06-06 17:44:12 -07:00
parent f01278616a
commit e0bfc4f91f
2 changed files with 4 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
View File

@ -820,12 +820,6 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
future.addListener((ChannelFutureListener) future1 ->
processGoAwayWriteResult(ctx, lastStreamId, errorCode, debugData, future1));
}
// if closeListener != null this means we have already initiated graceful closure. doGracefulShutdown will apply
// the gracefulShutdownTimeoutMillis on each invocation, however we only care to apply the timeout on the
// start of graceful shutdown.
if (errorCode == NO_ERROR.code() && closeListener == null) {
doGracefulShutdown(ctx, future, null);
}
return future;
}

8
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java
View File

@ -330,8 +330,8 @@ public class Http2FrameCodecTest {
channel.writeOutbound(goAwayFrame);
verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(7),
eq(NO_ERROR.code()), eq(expected), anyChannelPromise());
assertEquals(State.CLOSED, stream.state());
assertFalse(channel.isActive());
assertEquals(State.OPEN, stream.state());
assertTrue(channel.isActive());
expected.release();
debugData.release();
}
@ -395,8 +395,8 @@ public class Http2FrameCodecTest {
verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(Integer.MAX_VALUE),
eq(NO_ERROR.code()), eq(debugData), anyChannelPromise());
debugData.release();
assertEquals(State.CLOSED, stream.state());
assertFalse(channel.isActive());
assertEquals(State.OPEN, stream.state());
assertTrue(channel.isActive());
}
@Test