This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 1b10f5052b Make the AprLifecycleListener more robust 1b10f5052b is described below commit 1b10f5052bd8c9f07986227a050965bffaf47297 Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Jun 13 17:34:14 2024 +0100 Make the AprLifecycleListener more robust --- .../apache/catalina/core/AprLifecycleListener.java | 22 ++++--- .../catalina/core/TestAprLifecycleListener.java | 71 ++++++++++++++++++++++ webapps/docs/changelog.xml | 7 +++ 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java b/java/org/apache/catalina/core/AprLifecycleListener.java index 9a564af339..112b458f5a 100644 --- a/java/org/apache/catalina/core/AprLifecycleListener.java +++ b/java/org/apache/catalina/core/AprLifecycleListener.java @@ -23,7 +23,6 @@ import java.util.List; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleEvent; import org.apache.catalina.LifecycleListener; -import org.apache.catalina.Server; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jni.Library; @@ -35,11 +34,11 @@ import org.apache.tomcat.util.res.StringManager; /** * Implementation of <code>LifecycleListener</code> that will init and and destroy APR. * <p> - * This listener must only be nested within {@link Server} elements. + * Only one instance of the APR/Native library may be loaded per JVM. Loading multiple instances will trigger a JVM + * crash - typically when the Connectors are destroyed. This listener utilises reference counting to ensure that only + * one instance of the APR/Native library is loaded at any one time. * <p> - * <strong>Note</strong>: If you are running Tomcat in an embedded fashion and have more than one Server instance per - * JVM, this listener <em>must not</em> be added to the {@code Server} instances, but handled outside by the calling - * code which is bootstrapping the embedded Tomcat instances. Not doing so will lead to JVM crashes. + * If multiple listener configurations are found, only the first one initialised will be used. * * @since 4.1 */ @@ -99,6 +98,10 @@ public class AprLifecycleListener implements LifecycleListener { protected static final Object lock = new Object(); + // Guarded by lock + private static int referenceCount = 0; + + public static boolean isAprAvailable() { // https://bz.apache.org/bugzilla/show_bug.cgi?id=48613 if (AprStatus.isInstanceCreated()) { @@ -125,8 +128,9 @@ public class AprLifecycleListener implements LifecycleListener { if (Lifecycle.BEFORE_INIT_EVENT.equals(event.getType())) { synchronized (lock) { - if (!(event.getLifecycle() instanceof Server)) { - log.warn(sm.getString("listener.notServer", event.getLifecycle().getClass().getSimpleName())); + if (referenceCount++ != 0) { + // Already loaded (note test is performed before reference count is incremented) + return; } init(); for (String msg : initInfoLogMessages) { @@ -153,6 +157,10 @@ public class AprLifecycleListener implements LifecycleListener { } } else if (Lifecycle.AFTER_DESTROY_EVENT.equals(event.getType())) { synchronized (lock) { + if (--referenceCount != 0) { + // Still being used (note test is performed after reference count is decremented) + return; + } if (!AprStatus.isAprAvailable()) { return; } diff --git a/test/org/apache/catalina/core/TestAprLifecycleListener.java b/test/org/apache/catalina/core/TestAprLifecycleListener.java new file mode 100644 index 0000000000..946d7bddd1 --- /dev/null +++ b/test/org/apache/catalina/core/TestAprLifecycleListener.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.core; + +import org.junit.Test; + +import org.apache.catalina.startup.Tomcat; +import org.apache.tomcat.util.net.TesterSupport; +import org.apache.tomcat.util.net.openssl.OpenSSLImplementation; + +public class TestAprLifecycleListener { + + @Test + public void testMultipleServerInstancesUsingTomcatNativeLibrary01() throws Exception { + doTestMultipleServerInstancesUsingTomcatNativeLibrary(false); + } + + + @Test + public void testMultipleServerInstancesUsingTomcatNativeLibrary02() throws Exception { + doTestMultipleServerInstancesUsingTomcatNativeLibrary(true); + } + + + private void doTestMultipleServerInstancesUsingTomcatNativeLibrary(boolean reverseShutdownOrder) throws Exception { + Tomcat tomcat1 = new Tomcat(); + tomcat1.setPort(0); + TesterSupport.initSsl(tomcat1); + TesterSupport.configureSSLImplementation(tomcat1, OpenSSLImplementation.class.getName(), true); + tomcat1.init(); + + Tomcat tomcat2 = new Tomcat(); + tomcat2.setPort(0); + TesterSupport.initSsl(tomcat2); + TesterSupport.configureSSLImplementation(tomcat2, OpenSSLImplementation.class.getName(), true); + tomcat2.init(); + + // Start 1, then 2 + tomcat1.start(); + tomcat2.start(); + + if (reverseShutdownOrder) { + // Stop and destroy 2 then 1 + tomcat2.stop(); + tomcat2.destroy(); + } + + tomcat1.stop(); + tomcat1.destroy(); + + if (!reverseShutdownOrder) { + // Stop and destroy 1 then 2 + tomcat2.stop(); + tomcat2.destroy(); + } +} +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 05dae26581..dce08e2919 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -145,6 +145,13 @@ when the context is configured to use a bloom filter. Based on pull request <pr>730</pr> provided by bergander. (markt) </fix> + <add> + Introduce reference counting so the <code>AprLifecycleListener</code> + is more robust. This particularly targets more complex embedded + configurations with multiple server instances with independent + lifecycles where more than one server instance requires the + <code>AprLifecycleListener</code>. (markt) + </add> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org