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.

Author: normanmaurer

handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java

@@ -295,10 +295,8 @@ public final boolean isClient() {
 
     @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;
     }
 
     /**

handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java

@@ -168,7 +168,7 @@ private enum HandshakeState {
     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];
 
@@ -1185,6 +1185,9 @@ private HandshakeStatus handshake() throws SSLException {
             throw exception;
         }
 
+        // 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) {
             // Check if we have a pending exception that was created during the handshake and if so throw it after

handler/src/main/java/io/netty/handler/ssl/SslHandler.java

@@ -429,6 +429,10 @@ public void handlerRemoved0(ChannelHandlerContext ctx) throws Exception {
             // 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