From ea5bc27c83004b0bba27874d839ac06753a53210 Mon Sep 17 00:00:00 2001
From: Chen Liu <chenliunju@gmail.com>
Date: Fri, 20 Aug 2021 09:50:19 -0700
Subject: [PATCH] Always release the sslEngine inside SslHandler's
 handlerRemoved0 (#11605)

Motivation:
Make SslHandler's handlerRemoved0 method release the sslEngine
even if it fails in the middle.
See details in https://github.com/netty/netty/issues/11595.

Modifications:
Wrap the release of sslEngine into a finally block.

Result:
The sslEngine would be released eventually.

Co-authored-by: Chen Liu <cliu@splunk.com>
---
 .../java/io/netty/handler/ssl/SslHandler.java | 47 +++++++++----------
 1 file changed, 23 insertions(+), 24 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 a2bb14541d..dd065f97e7 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
@@ -34,7 +34,6 @@ import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.DecoderException;
 import io.netty.handler.codec.UnsupportedMessageTypeException;
 import io.netty.util.ReferenceCountUtil;
-import io.netty.util.ReferenceCounted;
 import io.netty.util.concurrent.DefaultPromise;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.Future;
@@ -650,32 +649,32 @@ public class SslHandler extends ByteToMessageDecoder {
 
     @Override
     public void handlerRemoved0(ChannelHandlerContext ctx) throws Exception {
-        if (!pendingUnencryptedWrites.isEmpty()) {
-            // Check if queue is not empty first because create a new ChannelException is expensive
-            pendingUnencryptedWrites.releaseAndFailAll(ctx,
-                    new ChannelException("Pending write on removal of SslHandler"));
-        }
-        pendingUnencryptedWrites = null;
-
-        SSLHandshakeException cause = null;
-
-        // If the handshake is not done yet we should fail the handshake promise and notify the rest of the
-        // pipeline.
-        if (!handshakePromise.isDone()) {
-            cause = new SSLHandshakeException("SslHandler removed before handshake completed");
-            if (handshakePromise.tryFailure(cause)) {
-                ctx.fireUserEventTriggered(new SslHandshakeCompletionEvent(cause));
+        try {
+            if (!pendingUnencryptedWrites.isEmpty()) {
+                // Check if queue is not empty first because create a new ChannelException is expensive
+                pendingUnencryptedWrites.releaseAndFailAll(ctx,
+                  new ChannelException("Pending write on removal of SslHandler"));
             }
-        }
-        if (!sslClosePromise.isDone()) {
-            if (cause == null) {
+            pendingUnencryptedWrites = null;
+
+            SSLHandshakeException cause = null;
+
+            // If the handshake is not done yet we should fail the handshake promise and notify the rest of the
+            // pipeline.
+            if (!handshakePromise.isDone()) {
                 cause = new SSLHandshakeException("SslHandler removed before handshake completed");
+                if (handshakePromise.tryFailure(cause)) {
+                    ctx.fireUserEventTriggered(new SslHandshakeCompletionEvent(cause));
+                }
             }
-            notifyClosePromise(cause);
-        }
-
-        if (engine instanceof ReferenceCounted) {
-            ((ReferenceCounted) engine).release();
+            if (!sslClosePromise.isDone()) {
+                if (cause == null) {
+                    cause = new SSLHandshakeException("SslHandler removed before handshake completed");
+                }
+                notifyClosePromise(cause);
+            }
+        } finally {
+            ReferenceCountUtil.release(engine);
         }
     }