Guard usage with null check to prevent possible NPE during finalization. (#12434)

normanmaurer · web-flow · commit 27e0f5260004 · 2022-06-01T11:20:26.000+02:00
Motivation:

We need to add a null check to ensure we not throw a NPE if the finalizer of ReferenceCountedOpenSslEngine runs because of an OOME before we assign the field.

Modifications:

- Add null check
- Assign fields first

Result:

No more NPE in finalizer possible
diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
@@ -229,6 +229,9 @@ protected void deallocate() {
                                   int peerPort, boolean jdkCompatibilityMode, boolean leakDetection) {
         super(peerHost, peerPort);
         OpenSsl.ensureAvailability();
+        engineMap = context.engineMap;
+        enableOcsp = context.enableOcsp;
+        this.jdkCompatibilityMode = jdkCompatibilityMode;
         this.alloc = checkNotNull(alloc, "alloc");
         apn = (OpenSslApplicationProtocolNegotiator) context.applicationProtocolNegotiator();
         clientMode = context.isClient();
@@ -309,13 +312,11 @@ public List<byte[]> getStatusResponses() {
         } else {
             session = new DefaultOpenSslSession(context.sessionContext());
         }
-        engineMap = context.engineMap;
-        enableOcsp = context.enableOcsp;
+
         if (!context.sessionContext().useKeyManager()) {
             session.setLocalCertificate(context.keyCertChain);
         }
 
-        this.jdkCompatibilityMode = jdkCompatibilityMode;
         Lock readerLock = context.ctxLock.readLock();
         readerLock.lock();
         final long finalSsl;
@@ -561,7 +562,12 @@ public final synchronized long sslPointer() {
     public final synchronized void shutdown() {
         if (!destroyed) {
             destroyed = true;
-            engineMap.remove(ssl);
+            // Let's check if engineMap is null as it could be in theory if we throw an OOME during the construction of
+            // ReferenceCountedOpenSslEngine (before we assign the field). This is needed as shutdown() is called from
+            // the finalizer as well.
+            if (engineMap != null) {
+                engineMap.remove(ssl);
+            }
             SSL.freeSSL(ssl);
             ssl = networkBIO = 0;
 
