Guard against possible segfault when OpenSslContext is gc'ed and user still hold reference to OpenSslSessionContext / OpenSslSessionStats

Motivation:

When the OpenSslContext is gc'ed and the user still hold a reference to OpenSslSessionContext / OpenSslSessionStats it is possible to produce a segfault when calling
a method on any of these that tries to pass down the ctx pointer to the native methods. This is because the OpenSslContext finalizer will free the native pointer.

Modifications:

Change OpenSslSessionContext / OpenSslSessionContext to store a reference to OpenSslContext and so prevent the GC to collect it as long as the user has a reference to OpenSslSessionContext / OpenSslSessionContext.

Result:

No more sefault possible.

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

@@ -264,7 +264,7 @@ public final class OpenSslClientContext extends OpenSslContext {
                     throw new SSLException("unable to setup trustmanager", e);
                 }
             }
-            sessionContext = new OpenSslClientSessionContext(ctx);
+            sessionContext = new OpenSslClientSessionContext(this);
             success = true;
         } finally {
             if (!success) {
@@ -280,7 +280,7 @@ public final class OpenSslClientContext extends OpenSslContext {
 
     // No cache is currently supported for client side mode.
     private static final class OpenSslClientSessionContext extends OpenSslSessionContext {
-        private OpenSslClientSessionContext(long context) {
+        private OpenSslClientSessionContext(OpenSslContext context) {
             super(context);
         }
 

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

@@ -365,7 +365,10 @@ public abstract class OpenSslContext extends SslContext {
         return ctx;
     }
 
-    protected final void destroy() {
+    // IMPORTANT: This method must only be called from either the constructor or the finalizer as a user MUST never
+    //            get access to an OpenSslSessionContext after this method was called to prevent the user from
+    //            producing a segfault.
+    final void destroy() {
         synchronized (OpenSslContext.class) {
             if (ctx != 0) {
                 SSLContext.free(ctx);

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

@@ -433,7 +433,7 @@ public final class OpenSslServerContext extends OpenSslContext {
                     throw new SSLException("unable to setup trustmanager", e);
                 }
             }
-            sessionContext = new OpenSslServerSessionContext(ctx);
+            sessionContext = new OpenSslServerSessionContext(this);
             sessionContext.setSessionIdContext(ID);
             success = true;
         } finally {

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

@@ -23,7 +23,7 @@ import org.apache.tomcat.jni.SSLContext;
  * {@link OpenSslSessionContext} implementation which offers extra methods which are only useful for the server-side.
  */
 public final class OpenSslServerSessionContext extends OpenSslSessionContext {
-    OpenSslServerSessionContext(long context) {
+    OpenSslServerSessionContext(OpenSslContext context) {
         super(context);
     }
 
@@ -32,12 +32,12 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext {
         if (seconds < 0) {
             throw new IllegalArgumentException();
         }
-        SSLContext.setSessionCacheTimeout(context, seconds);
+        SSLContext.setSessionCacheTimeout(context.ctx, seconds);
     }
 
     @Override
     public int getSessionTimeout() {
-        return (int) SSLContext.getSessionCacheTimeout(context);
+        return (int) SSLContext.getSessionCacheTimeout(context.ctx);
     }
 
     @Override
@@ -45,23 +45,23 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext {
         if (size < 0) {
             throw new IllegalArgumentException();
         }
-        SSLContext.setSessionCacheSize(context, size);
+        SSLContext.setSessionCacheSize(context.ctx, size);
     }
 
     @Override
     public int getSessionCacheSize() {
-        return (int) SSLContext.getSessionCacheSize(context);
+        return (int) SSLContext.getSessionCacheSize(context.ctx);
     }
 
     @Override
     public void setSessionCacheEnabled(boolean enabled) {
         long mode = enabled ? SSL.SSL_SESS_CACHE_SERVER : SSL.SSL_SESS_CACHE_OFF;
-        SSLContext.setSessionCacheMode(context, mode);
+        SSLContext.setSessionCacheMode(context.ctx, mode);
     }
 
     @Override
     public boolean isSessionCacheEnabled() {
-        return SSLContext.getSessionCacheMode(context) == SSL.SSL_SESS_CACHE_SERVER;
+        return SSLContext.getSessionCacheMode(context.ctx) == SSL.SSL_SESS_CACHE_SERVER;
     }
 
     /**
@@ -74,6 +74,6 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext {
      * @return {@code true} if success, {@code false} otherwise.
      */
     public boolean setSessionIdContext(byte[] sidCtx) {
-        return SSLContext.setSessionIdContext(context, sidCtx);
+        return SSLContext.setSessionIdContext(context.ctx, sidCtx);
     }
 }

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

@@ -30,9 +30,13 @@ public abstract class OpenSslSessionContext implements SSLSessionContext {
     private static final Enumeration<byte[]> EMPTY = new EmptyEnumeration();
 
     private final OpenSslSessionStats stats;
-    final long context;
+    final OpenSslContext context;
 
-    OpenSslSessionContext(long context) {
+    // IMPORTANT: We take the OpenSslContext and not just the long (which points the native instance) to prevent
+    //            the GC to collect OpenSslContext as this would also free the pointer and so could result in a
+    //            segfault when the user calls any of the methods here that try to pass the pointer down to the native
+    //            level.
+    OpenSslSessionContext(OpenSslContext context) {
         this.context = context;
         stats = new OpenSslSessionStats(context);
     }
@@ -59,7 +63,7 @@ public abstract class OpenSslSessionContext implements SSLSessionContext {
         if (keys == null) {
             throw new NullPointerException("keys");
         }
-        SSLContext.setSessionTicketKeys(context, keys);
+        SSLContext.setSessionTicketKeys(context.ctx, keys);
     }
 
     /**
@@ -73,7 +77,7 @@ public abstract class OpenSslSessionContext implements SSLSessionContext {
         for (int i = 0; i < ticketKeys.length; i++) {
             ticketKeys[i] = keys[i].key;
         }
-        SSLContext.setSessionTicketKeys(context, ticketKeys);
+        SSLContext.setSessionTicketKeys(context.ctx, ticketKeys);
     }
 
     /**

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

@@ -25,9 +25,13 @@ import org.apache.tomcat.jni.SSLContext;
  */
 public final class OpenSslSessionStats {
 
-    private final long context;
+    private final OpenSslContext context;
 
-    OpenSslSessionStats(long context) {
+    // IMPORTANT: We take the OpenSslContext and not just the long (which points the native instance) to prevent
+    //            the GC to collect OpenSslContext as this would also free the pointer and so could result in a
+    //            segfault when the user calls any of the methods here that try to pass the pointer down to the native
+    //            level.
+    OpenSslSessionStats(OpenSslContext context) {
         this.context = context;
     }
 
@@ -35,49 +39,49 @@ public final class OpenSslSessionStats {
      * Returns the current number of sessions in the internal session cache.
      */
     public long number() {
-        return SSLContext.sessionNumber(context);
+        return SSLContext.sessionNumber(context.ctx);
     }
 
     /**
      * Returns the number of started SSL/TLS handshakes in client mode.
      */
     public long connect() {
-        return SSLContext.sessionConnect(context);
+        return SSLContext.sessionConnect(context.ctx);
     }
 
     /**
      * Returns the number of successfully established SSL/TLS sessions in client mode.
      */
     public long connectGood() {
-        return SSLContext.sessionConnectGood(context);
+        return SSLContext.sessionConnectGood(context.ctx);
     }
 
     /**
      * Returns the number of start renegotiations in client mode.
      */
     public long connectRenegotiate() {
-        return SSLContext.sessionConnectRenegotiate(context);
+        return SSLContext.sessionConnectRenegotiate(context.ctx);
     }
 
     /**
      * Returns the number of started SSL/TLS handshakes in server mode.
      */
     public long accept() {
-        return SSLContext.sessionAccept(context);
+        return SSLContext.sessionAccept(context.ctx);
     }
 
     /**
      * Returns the number of successfully established SSL/TLS sessions in server mode.
      */
     public long acceptGood() {
-        return SSLContext.sessionAcceptGood(context);
+        return SSLContext.sessionAcceptGood(context.ctx);
     }
 
     /**
      * Returns the number of start renegotiations in server mode.
      */
     public long acceptRenegotiate() {
-        return SSLContext.sessionAcceptRenegotiate(context);
+        return SSLContext.sessionAcceptRenegotiate(context.ctx);
     }
 
     /**
@@ -86,14 +90,14 @@ public final class OpenSslSessionStats {
      * external cache is counted as a hit.
      */
     public long hits() {
-        return SSLContext.sessionHits(context);
+        return SSLContext.sessionHits(context.ctx);
     }
 
     /**
      * Returns the number of successfully retrieved sessions from the external session cache in server mode.
      */
     public long cbHits() {
-        return SSLContext.sessionCbHits(context);
+        return SSLContext.sessionCbHits(context.ctx);
     }
 
     /**
@@ -101,7 +105,7 @@ public final class OpenSslSessionStats {
      * in server mode.
      */
     public long misses() {
-        return SSLContext.sessionMisses(context);
+        return SSLContext.sessionMisses(context.ctx);
     }
 
     /**
@@ -110,13 +114,13 @@ public final class OpenSslSessionStats {
      * count.
      */
     public long timeouts() {
-        return SSLContext.sessionTimeouts(context);
+        return SSLContext.sessionTimeouts(context.ctx);
     }
 
     /**
      * Returns the number of sessions that were removed because the maximum session cache size was exceeded.
      */
     public long cacheFull() {
-        return SSLContext.sessionCacheFull(context);
+        return SSLContext.sessionCacheFull(context.ctx);
     }
 }