Skip to content

Commit

Permalink
Fix a bug where a connection is not closed immediatley when HTTP/2 PI…
Browse files Browse the repository at this point in the history
…NG write fails (#2699)

Motivation:

If a connection is closed unexpectedly due to a network problem,
the remote peer does not receive closing signals such as GOAWAY frame and
the connection is alive until cleaned up due to request timeout, idle timeout, or PING failure.

When PING write fails, KeepAliveHandler tries to close the connection, which takes as long as the idle timeout.
Because the idle timeout is configured as `gracefulShutdownTimeoutMillis` for Netty Http2ConnectionHandler.
https://github.com/line/armeria/blob/fec9fb833275eba59eb9afaf25fc8b7c28132f80/core/src/main/java/com/linecorp/armeria/server/Http2ServerConnectionHandler.java#L56-L62

Modifications:

* Close a connection immediately if KeepAliveHandler is closing.

Result:
A dead connection is cleaned up immediately when PING write fails.
  • Loading branch information
ikhoon committed May 12, 2020
1 parent 261a548 commit 424fb98
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 2 deletions.
Expand Up @@ -101,7 +101,9 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception {

@Override
protected boolean needsImmediateDisconnection() {
return clientFactory.isClosing() || responseDecoder.goAwayHandler().receivedErrorGoAway();
return clientFactory.isClosing() ||
responseDecoder.goAwayHandler().receivedErrorGoAway() ||
keepAliveHandler.isClosing();
}

@Override
Expand Down
Expand Up @@ -172,6 +172,10 @@ public final void onPing() {
cancelFutures();
}

public final boolean isClosing() {
return pingState == PingState.SHUTDOWN;
}

protected abstract ChannelFuture writePing(ChannelHandlerContext ctx);

protected abstract boolean pingResetsPreviousPing();
Expand Down
Expand Up @@ -67,7 +67,9 @@ final class Http2ServerConnectionHandler extends AbstractHttp2ConnectionHandler

@Override
protected boolean needsImmediateDisconnection() {
return gracefulShutdownSupport.isShuttingDown() || requestDecoder.goAwayHandler().receivedErrorGoAway();
return gracefulShutdownSupport.isShuttingDown() ||
requestDecoder.goAwayHandler().receivedErrorGoAway() ||
keepAliveHandler.isClosing();
}

@Override
Expand Down

0 comments on commit 424fb98

Please sign in to comment.