Fix APR crash on shutdown

Author: markt-asf

java/org/apache/catalina/core/AprLifecycleListener.java

@@ -19,6 +19,7 @@
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.locks.Lock;
 
 import org.apache.catalina.Lifecycle;
 import org.apache.catalina.LifecycleEvent;
@@ -99,18 +100,20 @@ public class AprLifecycleListener implements LifecycleListener {
 
     private static final int FIPS_OFF = 0;
 
-    protected static final Object lock = new Object();
-
-    // Guarded by lock
+    // Guarded APRStatus.getStatusLock()
     private static int referenceCount = 0;
     private boolean instanceInitialized = false;
 
 
     public static boolean isAprAvailable() {
         // https://bz.apache.org/bugzilla/show_bug.cgi?id=48613
         if (org.apache.tomcat.jni.AprStatus.isInstanceCreated()) {
-            synchronized (lock) {
+            Lock writeLock = org.apache.tomcat.jni.AprStatus.getStatusLock().writeLock();
+            writeLock.lock();
+            try {
                 init();
+            } finally {
+                writeLock.unlock();
             }
         }
         return org.apache.tomcat.jni.AprStatus.isAprAvailable();
@@ -131,7 +134,9 @@ public AprLifecycleListener() {
     public void lifecycleEvent(LifecycleEvent event) {
 
         if (Lifecycle.BEFORE_INIT_EVENT.equals(event.getType())) {
-            synchronized (lock) {
+            Lock writeLock = org.apache.tomcat.jni.AprStatus.getStatusLock().writeLock();
+            writeLock.lock();
+            try {
                 instanceInitialized = true;
                 if (!(event.getLifecycle() instanceof Server)) {
                     log.warn(sm.getString("listener.notServer", event.getLifecycle().getClass().getSimpleName()));
@@ -161,9 +166,13 @@ public void lifecycleEvent(LifecycleEvent event) {
                     log.fatal(e.getMessage(), e);
                     throw e;
                 }
+            } finally {
+                writeLock.unlock();
             }
         } else if (Lifecycle.AFTER_DESTROY_EVENT.equals(event.getType())) {
-            synchronized (lock) {
+            Lock writeLock = org.apache.tomcat.jni.AprStatus.getStatusLock().writeLock();
+            writeLock.lock();
+            try {
                 // Instance may get destroyed without ever being initialized
                 if (instanceInitialized) {
                     referenceCount--;
@@ -182,6 +191,8 @@ public void lifecycleEvent(LifecycleEvent event) {
                     ExceptionUtils.handleThrowable(throwable);
                     log.warn(sm.getString("aprListener.aprDestroy"), throwable);
                 }
+            } finally {
+                writeLock.unlock();
             }
         }
 

java/org/apache/tomcat/jni/AprStatus.java

@@ -16,6 +16,8 @@
  */
 package org.apache.tomcat.jni;
 
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
 /**
  * Holds APR status without the need to load other classes.
  */
@@ -25,6 +27,7 @@ public class AprStatus {
     private static volatile boolean useOpenSSL = true;
     private static volatile boolean instanceCreated = false;
     private static volatile int openSSLVersion = 0;
+    private static ReentrantReadWriteLock statusLock = new ReentrantReadWriteLock();
 
     public static boolean isAprInitialized() {
         return aprInitialized;
@@ -71,4 +74,16 @@ public static int getOpenSSLVersion() {
     public static void setOpenSSLVersion(int openSSLVersion) {
         AprStatus.openSSLVersion = openSSLVersion;
     }
+
+    /**
+     * Code that changes the status of the APR library MUST hold the write lock while making any changes.
+     * <p>
+     * Code that needs the status to be consistent for an operation must hold the read lock for the duration of that
+     * operation.
+     *
+     * @return The read/write lock for APR library status
+     */
+    public static ReentrantReadWriteLock getStatusLock() {
+        return statusLock;
+    }
 }

java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java

@@ -33,6 +33,7 @@
 import java.util.Base64;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.locks.Lock;
 
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLEngine;
@@ -46,6 +47,7 @@
 
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.jni.AprStatus;
 import org.apache.tomcat.jni.Pool;
 import org.apache.tomcat.jni.SSL;
 import org.apache.tomcat.jni.SSLConf;
@@ -592,14 +594,33 @@ public X509Certificate[] getAcceptedIssuers() {
     private record OpenSSLState(long aprPool, long cctx, long ctx) implements Runnable {
         @Override
         public void run() {
-            if (ctx != 0) {
-                SSLContext.free(ctx);
-            }
-            if (cctx != 0) {
-                SSLConf.free(cctx);
-            }
-            if (aprPool != 0) {
-                Pool.destroy(aprPool);
+            /*
+             * During shutdown there is a possibility that both the cleaner and the APR library termination code try and
+             * free these resources. If both call free, there will be a JVM crash.
+             *
+             * If the cleaner frees the resources, the APR library termination won't try free them as well.
+             *
+             * If the APR library termination frees the resources, the cleaner MUST NOT attempt to do so.
+             *
+             * The locks and checks below ensure that a) the cleaner only runs if the APR library has not yet been
+             * terminated and that the APR library status will not change while the cleaner is running.
+             */
+            Lock readLock = AprStatus.getStatusLock().readLock();
+            readLock.lock();
+            try {
+                if (AprStatus.isAprInitialized()) {
+                    if (ctx != 0) {
+                        SSLContext.free(ctx);
+                    }
+                    if (cctx != 0) {
+                        SSLConf.free(cctx);
+                    }
+                    if (aprPool != 0) {
+                        Pool.destroy(aprPool);
+                    }
+                }
+            } finally {
+                readLock.unlock();
             }
         }
     }