This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0f5fee193af89af53bba72fbc6477aac74af6d6c Author: Mark Thomas <[email protected]> AuthorDate: Fri Oct 31 15:17:55 2025 +0000 Fix APR crash on shutdown --- .../apache/catalina/core/AprLifecycleListener.java | 23 ++++++++++---- java/org/apache/tomcat/jni/AprStatus.java | 15 +++++++++ .../tomcat/util/net/openssl/OpenSSLContext.java | 37 +++++++++++++++++----- webapps/docs/changelog.xml | 4 +++ 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java b/java/org/apache/catalina/core/AprLifecycleListener.java index c8d6976ec5..bf047c362d 100644 --- a/java/org/apache/catalina/core/AprLifecycleListener.java +++ b/java/org/apache/catalina/core/AprLifecycleListener.java @@ -19,6 +19,7 @@ package org.apache.catalina.core; 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,9 +100,7 @@ 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; @@ -109,8 +108,12 @@ public class AprLifecycleListener implements LifecycleListener { 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 class AprLifecycleListener implements LifecycleListener { 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 class AprLifecycleListener implements LifecycleListener { 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 class AprLifecycleListener implements LifecycleListener { ExceptionUtils.handleThrowable(throwable); log.warn(sm.getString("aprListener.aprDestroy"), throwable); } + } finally { + writeLock.unlock(); } } diff --git a/java/org/apache/tomcat/jni/AprStatus.java b/java/org/apache/tomcat/jni/AprStatus.java index d255b2ba76..5b463afd61 100644 --- a/java/org/apache/tomcat/jni/AprStatus.java +++ b/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 class AprStatus { 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; + } } diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java index 76cf4f4f74..992c069fd4 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java @@ -33,6 +33,7 @@ import java.util.Arrays; 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 javax.net.ssl.X509TrustManager; 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; @@ -609,14 +611,33 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { @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(); } } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 97c23314ce..48f39f5130 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -127,6 +127,10 @@ <bug>69866</bug>: Fix a memory leak when using a trust store with the OpenSSL provider. Pull request <pr>912</pr> by aogburn. (markt) </fix> + <fix> + Fix potential crash on shutdown when a Connector depends on the Tomcat + Native library. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
