From f2eb106f1ea818eb7bafc68797b988a118781dec Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Mon, 4 Jan 2016 21:22:48 +0100
Subject: [PATCH] Ensure we only add OpenSslEngine to the OpenSslEngineMap when
 handshake is started

Motivation:

We need to ensure we only add the OpenSslEngine to the OpenSslEngineMap when the handshake is started as otherwise we may produce a memory leak when the OpenSslEngine is created but not actually used. This can for example happen if we encounter a connection refused from the remote peer. In this case we will never remove the OpenSslEngine from the OpenSslEngineMap and so it will never be collected (as we hold a reference). This has as affect that the finalizer will never be run as well.

Modifications:

- Lazy add the OpenSslEngine to the OpenSslEngineMap to elimate possible leak.
- Call OpenSslEngine.shutdown() when SslHandler is removed from the ChannelPipeline to free memory asap in all cases.

Result:

No more memory leak with OpenSslEngine if connection is refused.
---
 .../src/main/java/io/netty/handler/ssl/OpenSslContext.java  | 4 +---
 .../src/main/java/io/netty/handler/ssl/OpenSslEngine.java   | 6 +++++-
 handler/src/main/java/io/netty/handler/ssl/SslHandler.java  | 4 ++++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java
index 0a46532a66..88e25daf3e 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java
@@ -295,10 +295,8 @@ public abstract class OpenSslContext extends SslContext {
 
     @Override
     public final SSLEngine newEngine(ByteBufAllocator alloc, String peerHost, int peerPort) {
-        final OpenSslEngine engine = new OpenSslEngine(ctx, alloc, isClient(), sessionContext(), apn, engineMap,
+        return new OpenSslEngine(ctx, alloc, isClient(), sessionContext(), apn, engineMap,
                 rejectRemoteInitiatedRenegotiation, peerHost, peerPort, keyCertChain, clientAuth);
-        engineMap.add(engine);
-        return engine;
     }
 
     /**
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java
index 3b96f93dd0..7db1b34eb5 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java
@@ -168,7 +168,7 @@ public final class OpenSslEngine extends SSLEngine {
     private final OpenSslApplicationProtocolNegotiator apn;
     private final boolean rejectRemoteInitiatedRenegation;
     private final OpenSslSession session;
-    private final java.security.cert.Certificate[] localCerts;
+    private final Certificate[] localCerts;
     private final ByteBuffer[] singleSrcBuffer = new ByteBuffer[1];
     private final ByteBuffer[] singleDstBuffer = new ByteBuffer[1];
 
@@ -1161,6 +1161,10 @@ public final class OpenSslEngine extends SSLEngine {
             return FINISHED;
         }
         checkEngineClosed();
+
+        // Adding the OpenSslEngine to the OpenSslEngineMap so it can be used in the AbstractCertificateVerifier.
+        engineMap.add(this);
+
         int code = SSL.doHandshake(ssl);
         if (code <= 0) {
             checkPendingHandshakeException();
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 53e1d93e0f..599a37ed87 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
@@ -402,6 +402,10 @@ public class SslHandler extends ByteToMessageDecoder {
             // Check if queue is not empty first because create a new ChannelException is expensive
             pendingUnencryptedWrites.removeAndFailAll(new ChannelException("Pending write on removal of SslHandler"));
         }
+        if (engine instanceof OpenSslEngine) {
+            // Call shutdown so we ensure all the native memory is released asap
+            ((OpenSslEngine) engine).shutdown();
+        }
     }
 
     @Override