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

Clear scheduled timeout if channel is closed with incomplete WebSocket handshake (#10510)

Browse Source 
Motivation:

Consider a scenario when the client iniitiates a WebSocket handshake but before the handshake is complete,
the channel is closed due to some reason. In such scenario, the handshake timeout scheduled on the executor
is not cleared. The reason it is not cleared is because in such cases the handshakePromise is not completed.

Modifications:

This change completes the handshakePromise exceptinoally on channelInactive callback, if it has not been
completed so far. This triggers the callback on completion of the promise which clears the timeout scheduled
on the executor.

This PR also adds a test case which reproduces the scenario described above. The test case fails before the
fix is added and succeeds when the fix is applied.

Result:

After this change, the timeout scheduled on the executor will be cleared, thus freeing up thread resources.
This commit is contained in:
Divij Vaidya 2020-09-16 00:40:42 -07:00 committed by Norman Maurer
parent 7d985d26b4
commit 58911c1482
2 changed files with 44 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandshakeHandler.java
View File

@ -68,6 +68,15 @@ class WebSocketClientProtocolHandshakeHandler implements ChannelHandler {
applyHandshakeTimeout();
}
@Override
public void channelInactive(ChannelHandlerContext ctx) throws Exception {
if (!handshakePromise.isDone()) {
handshakePromise.tryFailure(new WebSocketHandshakeException("channel closed with handshake in progress"));
}
ctx.fireChannelInactive();
}
@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (!(msg instanceof FullHttpResponse)) {

38
codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java
View File

@ -186,6 +186,40 @@ public class WebSocketHandshakeHandOverTest {
}
}
/**
* Tests a scenario when channel is closed while the handshake is in progress. Validates that the handshake
* future is notified in such cases.
*/
@Test
public void testHandshakeFutureIsNotifiedOnChannelClose() throws Exception {
EmbeddedChannel clientChannel = createClientChannel(null);
EmbeddedChannel serverChannel = createServerChannel(null);
try {
// Start handshake from client to server but don't complete the handshake for the purpose of this test.
transferAllDataWithMerge(clientChannel, serverChannel);
final WebSocketClientProtocolHandler clientWsHandler =
clientChannel.pipeline().get(WebSocketClientProtocolHandler.class);
final WebSocketClientProtocolHandshakeHandler clientWsHandshakeHandler =
clientChannel.pipeline().get(WebSocketClientProtocolHandshakeHandler.class);
final ChannelHandlerContext ctx = clientChannel.pipeline().context(WebSocketClientProtocolHandler.class);
// Close the channel while the handshake is in progress. The channel could be closed before the handshake is
// complete due to a number of varied reasons. To reproduce the test scenario for this test case,
// we would manually close the channel.
clientWsHandler.close(ctx, ctx.newPromise());
// At this stage handshake is incomplete but the handshake future should be completed exceptionally since
// channel is closed.
assertTrue(clientWsHandshakeHandler.getHandshakeFuture().isDone());
} finally {
serverChannel.finishAndReleaseAll();
clientChannel.finishAndReleaseAll();
}
}
@Test(timeout = 10000)
public void testClientHandshakerForceClose() throws Exception {
final WebSocketClientHandshaker handshaker = WebSocketClientHandshakerFactory.newHandshaker(
@ -305,9 +339,7 @@ public class WebSocketHandshakeHandOverTest {
}
private static EmbeddedChannel createServerChannel(ChannelHandler handler) {
return new EmbeddedChannel(
new HttpServerCodec(),
new HttpObjectAggregator(8192),
return createServerChannel(
new WebSocketServerProtocolHandler("/test", "test-proto-1, test-proto-2", false),
handler);
}