From 2c965b14dbd6b6c24a079f02ab2f2a8f95970ada Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Tue, 27 Feb 2024 20:34:09 -0800
Subject: [PATCH] SSL: Hold the right monitor wheile running delegating task
 (#13875)

Motivation:

We need to ensure we hold the correct lock when we execute the
delegating task to guard against possible concurrent shutdown of the
engine.

Modifications:

- Fix incorrect synchronized block

Result:

Hold correct lock
---
 .../ssl/ReferenceCountedOpenSslEngine.java    | 23 +++++++++++--------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/handler/src/main/java/io/netty5/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty5/handler/ssl/ReferenceCountedOpenSslEngine.java
index f88002dce99..0a82fe85b9d 100644
--- a/handler/src/main/java/io/netty5/handler/ssl/ReferenceCountedOpenSslEngine.java
+++ b/handler/src/main/java/io/netty5/handler/ssl/ReferenceCountedOpenSslEngine.java
@@ -57,7 +57,6 @@
 import java.security.AlgorithmConstraints;
 import java.security.Principal;
 import java.security.cert.Certificate;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -1469,16 +1468,20 @@ public void run(Runnable runnable) {
         }
     }
 
-    private synchronized void runAndResetNeedTask(Runnable task) {
-        try {
-            if (isDestroyed()) {
-                // The engine was destroyed in the meantime, just return.
-                return;
+    private void runAndResetNeedTask(Runnable task) {
+        // We need to synchronize on the ReferenceCountedOpenSslEngine, we are sure the SSL object
+        // will not be freed by the user calling for example shutdown() concurrently.
+        synchronized (ReferenceCountedOpenSslEngine.this) {
+            try {
+                if (isDestroyed()) {
+                    // The engine was destroyed in the meantime, just return.
+                    return;
+                }
+                task.run();
+            } finally {
+                // The task was run, reset needTask to false so getHandshakeStatus() returns the correct value.
+                needTask = false;
             }
-            task.run();
-        } finally {
-            // The task was run, reset needTask to false so getHandshakeStatus() returns the correct value.
-            needTask = false;
         }
     }