From 6dbec4181cabe2d836e4a46b253d2f400fa296ac Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 1 Dec 2016 08:06:06 +0100 Subject: [PATCH] Ensure we not complete the same promise that may be failed because of outbound handler exception. Motivation: It's important that we do not pass in the original ChannelPromise to safeClose(...) as when flush(...) will throw an Exception it will be propagated to the AbstractChannelHandlerContext which will try to fail the promise because of this. This will then fail as it was already completed by safeClose(...). Modifications: Create a new ChannelPromise and pass it to safeClose(...). Result: No more confusing logs because of failing to fail the promise. --- .../main/java/io/netty/handler/ssl/SslHandler.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index edcd0ec2ca..29a3a2e90d 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -1292,7 +1292,16 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH try { flush(ctx, closeNotifyPromise); } finally { - safeClose(ctx, closeNotifyPromise, promise); + // It's important that we do not pass the original ChannelPromise to safeClose(...) as when flush(....) + // throws an Exception it will be propagated to the AbstractChannelHandlerContext which will try + // to fail the promise because of this. This will then fail as it was already completed by safeClose(...). + // We create a new ChannelPromise and try to notify the original ChannelPromise + // once it is complete. If we fail to do so we just ignore it as in this case it was failed already + // because of a propagated Exception. + // + // See https://github.com/netty/netty/issues/5931 + safeClose(ctx, closeNotifyPromise, ctx.newPromise().addListener( + new ChannelPromiseNotifier(false, promise))); } }