This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new ae448b3461 Rework Native library stability fix.
ae448b3461 is described below
commit ae448b346135f381ff695c16dd4a24bac184a3e5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Feb 9 12:35:59 2024 +0000
Rework Native library stability fix.
There are two aspects to this:
- Don't let the library temerinate while finalize is clearing
OpenSSLContext instances
- Don't allow finalize to clear OpenSSLContext instances once the
library generaiton that created that instance has terminated
---
.../apache/catalina/core/AprLifecycleListener.java | 5 +-
java/org/apache/tomcat/jni/Library.java | 53 ++++++++++++++++------
.../tomcat/util/net/openssl/OpenSSLContext.java | 26 +++++++----
test/org/apache/tomcat/util/net/TesterSupport.java | 1 -
4 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java
b/java/org/apache/catalina/core/AprLifecycleListener.java
index ffcc694e6f..604d01502a 100644
--- a/java/org/apache/catalina/core/AprLifecycleListener.java
+++ b/java/org/apache/catalina/core/AprLifecycleListener.java
@@ -175,14 +175,11 @@ public class AprLifecycleListener implements
LifecycleListener {
}
private static void terminateAPR() {
- Library.terminatePrepare();
- // Need to force GC here as some components do APR clean-up in
finalize()
- System.gc();
aprInitialized = false;
aprAvailable = false;
fipsModeActive = false;
sslInitialized = false; // Well we cleaned the pool in terminate.
- Library.terminate();
+ Library.threadSafeTerminate();
}
@SuppressWarnings("deprecation")
diff --git a/java/org/apache/tomcat/jni/Library.java
b/java/org/apache/tomcat/jni/Library.java
index a08dae4c07..e876c13d73 100644
--- a/java/org/apache/tomcat/jni/Library.java
+++ b/java/org/apache/tomcat/jni/Library.java
@@ -17,6 +17,10 @@
package org.apache.tomcat.jni;
import java.io.File;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
public final class Library {
@@ -30,7 +34,8 @@ public final class Library {
*/
private static Library _instance = null;
- private static volatile boolean initialized = false;
+ private static final AtomicLong generation = new AtomicLong(0);
+ private static final ReadWriteLock cleanUpLock = new
ReentrantReadWriteLock();
private Library() throws Exception {
boolean loaded = false;
@@ -81,21 +86,25 @@ public final class Library {
*/
private static native boolean initialize();
/**
- * Signal that Tomcat Native is about to be shutdown.
- * <p>
- * The main purpose of this flag is to allow instances that manage their
own APR root pools to determine if those
- * pools need to be explicitly cleaned up or if they will be / have been
cleaned up by the call to
- * {@link #terminate()}. The code needs to avoid multiple attempts to
clean up these pools else the Native code may
- * crash.
+ * Allows for thread safe termination when other threads may be attempting
clean-up concurrently with the current
+ * thread. Waits for any threads currently holding the clean-up lock to
release the lock and then calls
+ * {@link #terminate()}.
*/
- public static void terminatePrepare() {
- initialized = false;
+ public static void threadSafeTerminate() {
+ cleanUpLock.writeLock().lock();
+ try {
+ terminate();
+ } finally {
+ generation.incrementAndGet();
+ cleanUpLock.writeLock().unlock();
+ }
}
/**
* Destroys Tomcat Native's global APR pool. This has to be the last call
to TCN library. This will destroy any APR
* root pools that have not been explicitly destroyed.
* <p>
- * Callers of this method should call {@link #terminatePrepare()} before
calling this method.
+ * This method should only be used if the caller is certain that all other
threads have finished using the native
+ * library.
*/
public static native void terminate();
/* Internal function for loading APR Features */
@@ -263,13 +272,29 @@ public final class Library {
throw new UnsatisfiedLinkError("Missing threading support from
APR");
}
}
- initialized = initialize();
- return initialized;
+ return initialize();
+ }
+
+
+ public static boolean tryCleanUpLock(long cleanupGeneration) {
+ try {
+ boolean result = cleanUpLock.readLock().tryLock(0,
TimeUnit.SECONDS);
+ if (result && generation.get() == cleanupGeneration) {
+ return true;
+ }
+ cleanUpLock.readLock().unlock();
+ } catch (InterruptedException e) {
+ // Treated the same way as not getting the lock
+ }
+ return false;
}
+ public static long getGeneration() {
+ return generation.get();
+ }
- public static boolean isInitialized() {
- return initialized;
+ public static void returnCleanUpLock() {
+ cleanUpLock.readLock().unlock();
}
/**
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index b2878a8beb..06767e186d 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -87,6 +87,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
private final SSLHostConfigCertificate certificate;
private final List<String> negotiableProtocols;
private final long aprPool;
+ private final long aprGeneration;
private final AtomicInteger aprPoolDestroyed = new AtomicInteger(0);
// OpenSSLConfCmd context
protected final long cctx;
@@ -104,6 +105,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
this.sslHostConfig = certificate.getSSLHostConfig();
this.certificate = certificate;
aprPool = Pool.create(0);
+ aprGeneration = Library.getGeneration();
boolean success = false;
try {
// Create OpenSSLConfCmd context if used
@@ -190,15 +192,21 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public synchronized void destroy() {
// Guard against multiple destroyPools() calls triggered by
construction exception and finalize() later
- if (aprPoolDestroyed.compareAndSet(0, 1) && Library.isInitialized()) {
- if (ctx != 0) {
- SSLContext.free(ctx);
- }
- if (cctx != 0) {
- SSLConf.free(cctx);
- }
- if (aprPool != 0) {
- Pool.destroy(aprPool);
+ if (aprPoolDestroyed.compareAndSet(0, 1)) {
+ if (Library.tryCleanUpLock(aprGeneration)) {
+ try {
+ if (ctx != 0) {
+ SSLContext.free(ctx);
+ }
+ if (cctx != 0) {
+ SSLConf.free(cctx);
+ }
+ if (aprPool != 0) {
+ Pool.destroy(aprPool);
+ }
+ } finally {
+ Library.returnCleanUpLock();
+ }
}
}
}
diff --git a/test/org/apache/tomcat/util/net/TesterSupport.java
b/test/org/apache/tomcat/util/net/TesterSupport.java
index c0f908ffc9..568ea1ece8 100644
--- a/test/org/apache/tomcat/util/net/TesterSupport.java
+++ b/test/org/apache/tomcat/util/net/TesterSupport.java
@@ -101,7 +101,6 @@ public final class TesterSupport {
Library.initialize(null);
available = true;
version = SSL.version();
- Library.terminatePrepare();
Library.terminate();
} catch (Exception | LibraryNotFoundError ex) {
// Ignore
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
