From 2e92a2f5cdadb1c5932a75d3a84b9b89e77bde30 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 20 Mar 2018 15:27:07 +0100 Subject: [PATCH] Ensure we not schedule multiple timeouts for close notify Motivation: We should only schedule one timeout to wait for the close notify to be done. Modifications: Keep track of if we already scheduled a timeout for close notify and if so not schedule another one. Result: No duplicated timeouts. --- .../java/io/netty/handler/ssl/SslHandler.java | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) 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 c17a04445e..fb6fc92766 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -400,6 +400,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH private boolean needsFlush; private boolean outboundClosed; + private boolean closeNotify; private int packetLength; @@ -1591,16 +1592,27 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH try { flush(ctx, closeNotifyPromise); } finally { - // 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))); + if (!closeNotify) { + closeNotify = true; + // 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))); + } else { + /// We already handling the close_notify so just attach the promise to the sslClosePromise. + sslClosePromise.addListener(new FutureListener() { + @Override + public void operationComplete(Future future) { + promise.setSuccess(); + } + }); + } } }