Hi, On Wed, Jul 22, 2020 at 2:18 AM <fha...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository. > > fhanik pushed a commit to branch 9.0.x > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > commit f4dac6846c548144799b1c3f33aba4eb320a3413 > Author: Filip Hanik <fha...@pivotal.io> > AuthorDate: Mon Jul 13 12:43:55 2020 -0700 > > Avoid reflection for default instantiation > > (Most commonly used codepath) > > Avoid having to load APR classes in the Connector > > Ensure that IntrospectionUtils can call setProperty on > PersistentProviderRegistrations > --- > .../auth/message/config/AuthConfigFactory.java | 8 ++- > .../jaspic/PersistentProviderRegistrations.java | 5 +- > java/org/apache/catalina/connector/Connector.java | 14 ++--- > .../apache/catalina/core/AprLifecycleListener.java | 43 ++++++-------- > java/org/apache/catalina/core/AprStatus.java | 69 > ++++++++++++++++++++++ > java/org/apache/catalina/core/StandardHost.java | 4 +- > java/org/apache/catalina/loader/WebappLoader.java | 4 ++ > java/org/apache/catalina/startup/Tomcat.java | 8 ++- > 8 files changed, 119 insertions(+), 36 deletions(-) > > diff --git > a/java/javax/security/auth/message/config/AuthConfigFactory.java > b/java/javax/security/auth/message/config/AuthConfigFactory.java > index d98b2f2..b27235b 100644 > --- a/java/javax/security/auth/message/config/AuthConfigFactory.java > +++ b/java/javax/security/auth/message/config/AuthConfigFactory.java > @@ -72,8 +72,12 @@ public abstract class AuthConfigFactory { > // this class. Note that the Thread context > class loader > // should not be used since that would > trigger a memory leak > // in container environments. > - Class<?> clazz = Class.forName(className); > - return (AuthConfigFactory) > clazz.getConstructor().newInstance(); > + if > (className.equals("org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl")) > { > + return new > org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl(); > + } else { > + Class<?> clazz = Class.forName(className); > + return (AuthConfigFactory) > clazz.getConstructor().newInstance(); > + } > } > }); > } catch (PrivilegedActionException e) { > diff --git > a/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java > b/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java > index a1ba60c..cd75799 100644 > --- > a/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java > +++ > b/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java > @@ -41,7 +41,7 @@ import org.xml.sax.SAXException; > * Utility class for the loading and saving of JASPIC persistent provider > * registrations. > */ > -final class PersistentProviderRegistrations { > +public final class PersistentProviderRegistrations { > > private static final StringManager sm = > > StringManager.getManager(PersistentProviderRegistrations.class); > @@ -233,6 +233,9 @@ final class PersistentProviderRegistrations { > public void addProperty(Property property) { > properties.put(property.getName(), property.getValue()); > } > + public void setProperty(String name, String value) { > + addProperty(name, value); > + } > void addProperty(String name, String value) { > properties.put(name, value); > } > diff --git a/java/org/apache/catalina/connector/Connector.java > b/java/org/apache/catalina/connector/Connector.java > index b22ce95..1cc1580 100644 > --- a/java/org/apache/catalina/connector/Connector.java > +++ b/java/org/apache/catalina/connector/Connector.java > @@ -29,7 +29,7 @@ import org.apache.catalina.Globals; > import org.apache.catalina.LifecycleException; > import org.apache.catalina.LifecycleState; > import org.apache.catalina.Service; > -import org.apache.catalina.core.AprLifecycleListener; > +import org.apache.catalina.core.AprStatus; > import org.apache.catalina.util.LifecycleMBeanBase; > import org.apache.coyote.AbstractProtocol; > import org.apache.coyote.Adapter; > @@ -80,8 +80,8 @@ public class Connector extends LifecycleMBeanBase { > > > public Connector(String protocol) { > - boolean apr = AprLifecycleListener.isAprAvailable() && > - AprLifecycleListener.getUseAprConnector(); > + boolean apr = AprStatus.isAprAvailable() && > + AprStatus.getUseAprConnector(); > ProtocolHandler p = null; > try { > p = ProtocolHandler.create(protocol, apr); > @@ -625,7 +625,7 @@ public class Connector extends LifecycleMBeanBase { > * @return the Coyote protocol handler in use. > */ > public String getProtocol() { > - boolean apr = AprLifecycleListener.getUseAprConnector(); > + boolean apr = AprStatus.getUseAprConnector(); > if ((!apr && > org.apache.coyote.http11.Http11NioProtocol.class.getName().equals(protocolHandlerClassName)) > || (apr && > org.apache.coyote.http11.Http11AprProtocol.class.getName().equals(protocolHandlerClassName))) > { > return "HTTP/1.1"; > @@ -1016,15 +1016,15 @@ public class Connector extends LifecycleMBeanBase > { > setParseBodyMethods(getParseBodyMethods()); > } > > - if (protocolHandler.isAprRequired() && > !AprLifecycleListener.isInstanceCreated()) { > + if (protocolHandler.isAprRequired() && > !AprStatus.isInstanceCreated()) { > throw new > LifecycleException(sm.getString("coyoteConnector.protocolHandlerNoAprListener", > getProtocolHandlerClassName())); > } > - if (protocolHandler.isAprRequired() && > !AprLifecycleListener.isAprAvailable()) { > + if (protocolHandler.isAprRequired() && > !AprStatus.isAprAvailable()) { > throw new > LifecycleException(sm.getString("coyoteConnector.protocolHandlerNoAprLibrary", > getProtocolHandlerClassName())); > } > - if (AprLifecycleListener.isAprAvailable() && > AprLifecycleListener.getUseOpenSSL() && > + if (AprStatus.isAprAvailable() && AprStatus.getUseOpenSSL() && > protocolHandler instanceof AbstractHttp11JsseProtocol) { > AbstractHttp11JsseProtocol<?> jsseProtocolHandler = > (AbstractHttp11JsseProtocol<?>) protocolHandler; > diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java > b/java/org/apache/catalina/core/AprLifecycleListener.java > index 0bde68c..9dd2e93 100644 > --- a/java/org/apache/catalina/core/AprLifecycleListener.java > +++ b/java/org/apache/catalina/core/AprLifecycleListener.java > @@ -46,7 +46,6 @@ public class AprLifecycleListener > implements LifecycleListener { > > private static final Log log = > LogFactory.getLog(AprLifecycleListener.class); > - private static boolean instanceCreated = false; > /** > * Info messages during init() are cached until > Lifecycle.BEFORE_INIT_EVENT > * so that, in normal (non-error) cases, init() related log messages > appear > @@ -76,10 +75,6 @@ public class AprLifecycleListener > protected static String FIPSMode = "off"; // default off, valid only > when SSLEngine="on" > protected static String SSLRandomSeed = "builtin"; > protected static boolean sslInitialized = false; > - protected static boolean aprInitialized = false; > - protected static boolean aprAvailable = false; > - protected static boolean useAprConnector = false; > - protected static boolean useOpenSSL = true; > protected static boolean fipsModeActive = false; > > /** > @@ -102,16 +97,16 @@ public class AprLifecycleListener > > public static boolean isAprAvailable() { > //https://bz.apache.org/bugzilla/show_bug.cgi?id=48613 > - if (instanceCreated) { > + if (AprStatus.isInstanceCreated()) { > synchronized (lock) { > init(); > } > } > - return aprAvailable; > + return AprStatus.isAprAvailable(); > } > > public AprLifecycleListener() { > - instanceCreated = true; > + AprStatus.setInstanceCreated(true); > } > > // ---------------------------------------------- LifecycleListener > Methods > @@ -131,7 +126,7 @@ public class AprLifecycleListener > log.info(msg); > } > initInfoLogMessages.clear(); > - if (aprAvailable) { > + if (AprStatus.isAprAvailable()) { > try { > initializeSSL(); > } catch (Throwable t) { > @@ -151,7 +146,7 @@ public class AprLifecycleListener > } > } else if (Lifecycle.AFTER_DESTROY_EVENT.equals(event.getType())) > { > synchronized (lock) { > - if (!aprAvailable) { > + if (!AprStatus.isAprAvailable()) { > return; > } > try { > @@ -174,8 +169,8 @@ public class AprLifecycleListener > Method method = Class.forName("org.apache.tomcat.jni.Library") > .getMethod(methodName, (Class [])null); > method.invoke(null, (Object []) null); > - aprAvailable = false; > - aprInitialized = false; > + AprStatus.setAprAvailable(false); > + AprStatus.setAprInitialized(false); > sslInitialized = false; // Well we cleaned the pool in terminate. > fipsModeActive = false; > } > @@ -189,10 +184,10 @@ public class AprLifecycleListener > int rqver = TCN_REQUIRED_MAJOR * 1000 + TCN_REQUIRED_MINOR * 100 > + TCN_REQUIRED_PATCH; > int rcver = TCN_REQUIRED_MAJOR * 1000 + TCN_RECOMMENDED_MINOR * > 100 + TCN_RECOMMENDED_PV; > > - if (aprInitialized) { > + if (AprStatus.isAprInitialized()) { > return; > } > - aprInitialized = true; > + AprStatus.setAprInitialized(true); > > try { > Library.initialize(null); > @@ -255,10 +250,10 @@ public class AprLifecycleListener > Boolean.valueOf(Library.APR_HAS_RANDOM))); > > initInfoLogMessages.add(sm.getString("aprListener.config", > - Boolean.valueOf(useAprConnector), > - Boolean.valueOf(useOpenSSL))); > + Boolean.valueOf(AprStatus.getUseAprConnector()), > + Boolean.valueOf(AprStatus.getUseOpenSSL()))); > > - aprAvailable = true; > + AprStatus.setAprAvailable(true); > } > > private static void initializeSSL() throws Exception { > @@ -402,27 +397,27 @@ public class AprLifecycleListener > } > > public void setUseAprConnector(boolean useAprConnector) { > - if (useAprConnector != AprLifecycleListener.useAprConnector) { > - AprLifecycleListener.useAprConnector = useAprConnector; > + if (useAprConnector != AprStatus.getUseAprConnector()) { > + AprStatus.setUseAprConnector(useAprConnector); > } > } > > public static boolean getUseAprConnector() { > - return useAprConnector; > + return AprStatus.getUseAprConnector(); > } > > public void setUseOpenSSL(boolean useOpenSSL) { > - if (useOpenSSL != AprLifecycleListener.useOpenSSL) { > - AprLifecycleListener.useOpenSSL = useOpenSSL; > + if (useOpenSSL != AprStatus.getUseOpenSSL()) { > + AprStatus.setUseOpenSSL(useOpenSSL); > } > } > > public static boolean getUseOpenSSL() { > - return useOpenSSL; > + return AprStatus.getUseOpenSSL(); > } > > public static boolean isInstanceCreated() { > - return instanceCreated; > + return AprStatus.isInstanceCreated(); > } > > } > diff --git a/java/org/apache/catalina/core/AprStatus.java > b/java/org/apache/catalina/core/AprStatus.java > new file mode 100644 > index 0000000..38aaa6d > --- /dev/null > +++ b/java/org/apache/catalina/core/AprStatus.java > @@ -0,0 +1,69 @@ > +/* > + * 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; > + > +/** > + * Holds APR status. > + */ > +public class AprStatus { > + private static volatile boolean aprInitialized = false; > + private static volatile boolean aprAvailable = false; > + private static volatile boolean useAprConnector = false; > + private static volatile boolean useOpenSSL = true; > Is this a good default value ? OpenSSL should be used only when APR is available, no ? If APR is not available then JSSE should be used. Martin > + private static volatile boolean instanceCreated = false; > + > + > + public static boolean isAprInitialized() { > + return aprInitialized; > + } > + > + public static boolean isAprAvailable() { > + return aprAvailable; > + } > + > + public static boolean getUseAprConnector() { > + return useAprConnector; > + } > + > + public static boolean getUseOpenSSL() { > + return useOpenSSL; > + } > + > + public static boolean isInstanceCreated() { > + return instanceCreated; > + } > + > + public static void setAprInitialized(boolean aprInitialized) { > + AprStatus.aprInitialized = aprInitialized; > + } > + > + public static void setAprAvailable(boolean aprAvailable) { > + AprStatus.aprAvailable = aprAvailable; > + } > + > + public static void setUseAprConnector(boolean useAprConnector) { > + AprStatus.useAprConnector = useAprConnector; > + } > + > + public static void setUseOpenSSL(boolean useOpenSSL) { > + AprStatus.useOpenSSL = useOpenSSL; > + } > + > + public static void setInstanceCreated(boolean instanceCreated) { > + AprStatus.instanceCreated = instanceCreated; > + } > +} > diff --git a/java/org/apache/catalina/core/StandardHost.java > b/java/org/apache/catalina/core/StandardHost.java > index 48e9dfb..bf8a096 100644 > --- a/java/org/apache/catalina/core/StandardHost.java > +++ b/java/org/apache/catalina/core/StandardHost.java > @@ -42,6 +42,7 @@ import org.apache.catalina.LifecycleListener; > import org.apache.catalina.Valve; > import org.apache.catalina.loader.WebappClassLoaderBase; > import org.apache.catalina.util.ContextName; > +import org.apache.catalina.valves.ErrorReportValve; > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > import org.apache.tomcat.util.ExceptionUtils; > @@ -827,7 +828,8 @@ public class StandardHost extends ContainerBase > implements Host { > } > } > if(!found) { > - Valve valve = > + Valve valve = > ErrorReportValve.class.getName().equals(errorValve) ? > + new ErrorReportValve() : > (Valve) > Class.forName(errorValve).getConstructor().newInstance(); > getPipeline().addValve(valve); > } > diff --git a/java/org/apache/catalina/loader/WebappLoader.java > b/java/org/apache/catalina/loader/WebappLoader.java > index 2f6f395..0dc64ed 100644 > --- a/java/org/apache/catalina/loader/WebappLoader.java > +++ b/java/org/apache/catalina/loader/WebappLoader.java > @@ -507,6 +507,10 @@ public class WebappLoader extends LifecycleMBeanBase > private WebappClassLoaderBase createClassLoader() > throws Exception { > > + if > (ParallelWebappClassLoader.class.getName().equals(loaderClass)) { > + return new ParallelWebappClassLoader(parentClassLoader); > + } > + > Class<?> clazz = Class.forName(loaderClass); > WebappClassLoaderBase classLoader = null; > > diff --git a/java/org/apache/catalina/startup/Tomcat.java > b/java/org/apache/catalina/startup/Tomcat.java > index 8184585..0811492 100644 > --- a/java/org/apache/catalina/startup/Tomcat.java > +++ b/java/org/apache/catalina/startup/Tomcat.java > @@ -1011,6 +1011,7 @@ public class Tomcat { > * @return newly created {@link Context} > */ > private Context createContext(Host host, String url) { > + String defaultContextClass = StandardContext.class.getName(); > String contextClass = StandardContext.class.getName(); > if (host == null) { > host = this.getHost(); > @@ -1019,8 +1020,13 @@ public class Tomcat { > contextClass = ((StandardHost) host).getContextClass(); > } > try { > - return (Context) Class.forName(contextClass).getConstructor() > + if (defaultContextClass.equals(contextClass)) { > + return new StandardContext(); > + } else { > + return (Context) > Class.forName(contextClass).getConstructor() > .newInstance(); > + } > + > } catch (ReflectiveOperationException | IllegalArgumentException > | SecurityException e) { > throw new > IllegalArgumentException(sm.getString("tomcat.noContextClass", > contextClass, host, url), e); > } > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >