You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2024/03/13 14:30:09 UTC
[PR] Centralize initialization in `Provider` class (logging-log4j2)
ppkarwasz opened a new pull request, #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374
We move the code responsible for the instantiation of `LoggerContextFactory` and `ThreadContextMap` from the static entry points to the logging system (`LogManager` and `ThreadContext`) to the `Provider` class.
The `Provider` class is instantiated using `ServiceLoader`, so `log4j-core` 2.x and 3.x can reimplement the initialization process according to their own rules. E.g. `log4j-core` 3.x can use the DI to create an instance of `LoggerContextFactory` and `ThreadContextMap`.
The following modification were performed:
* a **new** system property `log4j.provider` was introduced,
* the old `log4j2.loggerContextFactory` has been deprecated and revised: if set it selects the first provider that uses the given `LoggerContextFactory`. Therefore it selects now both the context factory and thread context map implementations,
* private static configuration values were removed from `ThreadContextMap` implementations, helping test parallelisation,
* a distinct `NoOpThreadContextStack` implementation has been introduced.
This PR only includes tests for the first two items. The remaining points **should** be covered by existing tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "rgoers (via GitHub)" <gi...@apache.org>.
rgoers commented on PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#issuecomment-2173785816
I apologize for taking so long to look at this but while working on moving context related stuff to log4j-context-data I ran into this and it makes things much more difficult as it hard-wires a bunch of classes that should be internal/private into the Provider class. I would request that those changes be undone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1530270173
##########
log4j-api/src/main/java/org/apache/logging/log4j/util/ProviderUtil.java:
##########
@@ -163,6 +172,88 @@ static void lazyInit() {
}
}
+ /**
+ * Used to test the public {@link #getProvider()} method.
+ */
+ static Provider selectProvider(final PropertiesUtil properties) {
Review Comment:
I increased the test coverage with some smoke tests.
If users specify a faulty configuration, I just check that a warning or error appears and no exception is thrown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1530265806
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -20,39 +20,135 @@
import java.net.URL;
import java.util.Properties;
import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.simple.SimpleLoggerContextFactory;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.Constants;
+import org.apache.logging.log4j.util.Lazy;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.apache.logging.log4j.util.PropertiesUtil;
/**
- * Model class for a Log4j 2 provider. The properties in this class correspond to the properties used in a
- * {@code META-INF/log4j-provider.properties} file. Note that this class is automatically created by Log4j and should
- * not be used by providers.
+ * Service class used to bind the Log4j API with an implementation.
+ * <p>
+ * Implementors should register an implementation of this class with {@link java.util.ServiceLoader}.
+ * </p>
+ * <p>
+ * <strong>Deprecated:</strong> the automatic registration of providers from
+ * {@code META-INF/log4j-provider.properties} is supported for compatibility reasons. Support for this file will
+ * be dropped in a future version.
+ * </p>
*/
public class Provider {
+ protected static final String CURRENT_VERSION = "2.6.0";
+
/**
* Property name to set for a Log4j 2 provider to specify the priority of this implementation.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String FACTORY_PRIORITY = "FactoryPriority";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.ThreadContextMap}.
+ * Property name to set to the implementation of {@link ThreadContextMap}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String THREAD_CONTEXT_MAP = "ThreadContextMap";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.LoggerContextFactory}.
+ * Property name to set to the implementation of {@link LoggerContextFactory}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String LOGGER_CONTEXT_FACTORY = "LoggerContextFactory";
- private static final Integer DEFAULT_PRIORITY = Integer.valueOf(-1);
+ /**
+ * System property used to specify the class name of the provider to use.
+ * @since 2.24.0
+ */
+ public static final String PROVIDER_PROPERTY_NAME = "log4j.provider";
+
+ // Bundled context map implementations
+ private static final String BASE = "org.apache.logging.log4j.internal.map.";
+
+ /**
+ * Constant used to disable the {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String NO_OP_CONTEXT_MAP = BASE + "NoOp";
+
+ /**
+ * Constant used to select a web application-safe implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation only binds JRE classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String WEB_APP_CONTEXT_MAP = BASE + "WebApp";
+
+ /**
+ * Constant used to select a copy-on-write implementation of {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String COPY_ON_WRITE_CONTEXT_MAP = BASE + "CopyOnWrite";
+
+ /**
+ * Constant used to select a garbage-free implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation must ensure that common operations don't create new object instances. The drawback is
+ * the necessity to bind custom classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String GARBAGE_FREE_CONTEXT_MAP = BASE + "GarbageFree";
Review Comment:
I marked the constants as `protected`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1527192823
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
Review Comment:
This is actually one of the two methods that implementations should implement.
Due to the long history of the class, there are other methods:
- `#getClassName()` gives the name of the class, but then Log4j API must guess the classloader to use to load it,
- `#loadLoggerContextFactory()` returns a class, but unless I am mistaken JPMS will not allow Log4j API to instantiate the class unless it is `public` and in an exported package,
I introduced this method so that implementations can instantiate it in any way they need. E.g. Log4j Core 3.x uses:
```java
new Log4jContextFactory(instanceFactory);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1527118053
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java:
##########
@@ -43,40 +43,30 @@ public class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString
private final boolean useMap;
private final ThreadLocal<Map<String, String>> localMap;
- private static boolean inheritableMap;
-
- static {
- init();
- }
-
- // LOG4J2-479: by default, use a plain ThreadLocal, only use InheritableThreadLocal if configured.
- // (This method is package protected for JUnit tests.)
- static ThreadLocal<Map<String, String>> createThreadLocalMap(final boolean isMapEnabled) {
- if (inheritableMap) {
- return new InheritableThreadLocal<Map<String, String>>() {
- @Override
- protected Map<String, String> childValue(final Map<String, String> parentValue) {
- return parentValue != null && isMapEnabled //
- ? Collections.unmodifiableMap(new HashMap<>(parentValue)) //
- : null;
- }
- };
- }
- // if not inheritable, return plain ThreadLocal with null as initial value
- return new ThreadLocal<>();
- }
-
- static void init() {
- inheritableMap = PropertiesUtil.getProperties().getBooleanProperty(INHERITABLE_MAP);
- }
-
public DefaultThreadContextMap() {
this(true);
}
+ /**
+ * @deprecated Since 2.24.0 use the default constructor or {@link NoOpThreadContextMap} instead.
+ */
+ @Deprecated
public DefaultThreadContextMap(final boolean useMap) {
+ this(useMap, PropertiesUtil.getProperties());
+ }
+
+ DefaultThreadContextMap(final boolean useMap, final PropertiesUtil properties) {
this.useMap = useMap;
- this.localMap = createThreadLocalMap(useMap);
+ localMap = properties.getBooleanProperty(INHERITABLE_MAP)
+ ? new InheritableThreadLocal<Map<String, String>>() {
+ @Override
+ protected Map<String, String> childValue(final Map<String, String> parentValue) {
+ return parentValue != null && useMap
+ ? Collections.unmodifiableMap(new HashMap<>(parentValue))
+ : null;
+ }
+ }
+ : new ThreadLocal<Map<String, String>>();
Review Comment:
`localMap` must be a `ThreadLocal`, ;-)
If `!useMap` I could probably set `localMap` to `null`, but our code (see `Provider`) does not use `DefaultThreadContextMap` in such a case. I leave the `useMap` parameter only for backward compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1530267750
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
Review Comment:
I made the method private. Implementors should override `getLoggerContextFactory`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1527193084
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
Review Comment:
I wanted to stress out the `LoggerContextFactory` is a singleton.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1525322283
##########
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.logging.log4j.spi;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.time.Duration;
+import java.util.Properties;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.stream.Stream;
+import org.apache.logging.log4j.util.PropertiesUtil;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class ThreadContextMapTest {
+
+ static Stream<ThreadContextMap> defaultMaps() {
+ return Stream.of(
+ new DefaultThreadContextMap(),
+ new CopyOnWriteSortedArrayThreadContextMap(),
+ new GarbageFreeSortedArrayThreadContextMap());
+ }
+
+ static Stream<ThreadContextMap> inheritableMaps() {
+ final Properties props = new Properties();
+ props.setProperty("log4j2.isThreadContextMapInheritable", "true");
+ final PropertiesUtil util = new PropertiesUtil(props);
+ return Stream.of(
+ new DefaultThreadContextMap(true, util),
+ new CopyOnWriteSortedArrayThreadContextMap(util),
+ new GarbageFreeSortedArrayThreadContextMap(util));
+ }
+
+ @ParameterizedTest
+ @MethodSource("defaultMaps")
+ void threadLocalNotInheritableByDefault(final ThreadContextMap contextMap) {
+ contextMap.put("key", "threadLocalNotInheritableByDefault");
+ final ExecutorService executorService = Executors.newSingleThreadExecutor();
+ try {
+ assertThat(executorService.submit(() -> contextMap.get("key")))
+ .succeedsWithin(Duration.ofSeconds(1))
+ .isEqualTo(null);
+ } finally {
+ executorService.shutdown();
+ }
Review Comment:
This block is (almost) identical to the one below. You can implement a single `verifyThreadContextValueAccessedFromAnotherThread(ThreadContextMap contextMap, String expectedValue)` method instead and use it for both tests.
##########
log4j-api/src/main/java/org/apache/logging/log4j/util/ProviderUtil.java:
##########
@@ -163,6 +172,88 @@ static void lazyInit() {
}
}
+ /**
+ * Used to test the public {@link #getProvider()} method.
+ */
+ static Provider selectProvider(final PropertiesUtil properties) {
Review Comment:
I presume you sufficiently covered this method with tests. :see_no_evil:
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java:
##########
@@ -43,40 +43,30 @@ public class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString
private final boolean useMap;
private final ThreadLocal<Map<String, String>> localMap;
- private static boolean inheritableMap;
-
- static {
- init();
- }
-
- // LOG4J2-479: by default, use a plain ThreadLocal, only use InheritableThreadLocal if configured.
- // (This method is package protected for JUnit tests.)
- static ThreadLocal<Map<String, String>> createThreadLocalMap(final boolean isMapEnabled) {
- if (inheritableMap) {
- return new InheritableThreadLocal<Map<String, String>>() {
- @Override
- protected Map<String, String> childValue(final Map<String, String> parentValue) {
- return parentValue != null && isMapEnabled //
- ? Collections.unmodifiableMap(new HashMap<>(parentValue)) //
- : null;
- }
- };
- }
- // if not inheritable, return plain ThreadLocal with null as initial value
- return new ThreadLocal<>();
- }
-
- static void init() {
- inheritableMap = PropertiesUtil.getProperties().getBooleanProperty(INHERITABLE_MAP);
- }
-
public DefaultThreadContextMap() {
this(true);
}
+ /**
+ * @deprecated Since 2.24.0 use the default constructor or {@link NoOpThreadContextMap} instead.
Review Comment:
Why don't we mark `NoOpThreadContextMap` as deprecated too and remove the suggestion of using it? That is, just like we do in `DefaultThreadContextStack(boolean useStack)`. In a future major release, we can make `NoOpThreadContextMap` private.
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
+ return loggerContextFactoryLazy.get();
+ }
+
+ /**
+ * Gets the class name of the {@link ThreadContextMap} implementation of this Provider.
+ * <p>
+ * This method should return one of the internal implementations:
+ * <ol>
+ * <li>{@code null} if {@link #loadThreadContextMap} is implemented,</li>
+ * <li>{@link #NO_OP_CONTEXT_MAP},</li>
+ * <li>{@link #WEB_APP_CONTEXT_MAP},</li>
+ * <li>{@link #COPY_ON_WRITE_CONTEXT_MAP},</li>
+ * <li>{@link #GARBAGE_FREE_CONTEXT_MAP}.</li>
+ * </ol>
+ * </p>
* @return the class name of a ThreadContextMap implementation
+ * @see #loadThreadContextMap()
*/
public String getThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass.getName();
}
- return threadContextMap;
+ // Field value
+ if (threadContextMap != null) {
+ return threadContextMap;
+ }
+ // Properties
+ final PropertiesUtil props = PropertiesUtil.getProperties();
+ if (props.getBooleanProperty(DISABLE_CONTEXT_MAP) || props.getBooleanProperty(DISABLE_THREAD_CONTEXT)) {
+ return NO_OP_CONTEXT_MAP;
+ }
+ final String threadContextMapClass = props.getStringProperty(THREAD_CONTEXT_MAP_PROPERTY);
+ if (threadContextMapClass != null) {
+ return threadContextMapClass;
+ }
+ // Default based on properties
+ if (Constants.ENABLE_THREADLOCALS) {
+ return props.getBooleanProperty(GC_FREE_THREAD_CONTEXT_PROPERTY)
+ ? GC_FREE_THREAD_CONTEXT_PROPERTY
+ : COPY_ON_WRITE_CONTEXT_MAP;
+ }
+ return WEB_APP_CONTEXT_MAP;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.ThreadContextMap} class specified by this Provider.
+ * Loads the {@link ThreadContextMap} class specified by this Provider.
*
- * @return the ThreadContextMap implementation class or {@code null} if there was an error loading it
+ * @return the {@code ThreadContextMap} implementation class or {@code null} if unspecified or a loading error
+ * occurred.
+ * @see #createThreadContextMap()
*/
public Class<? extends ThreadContextMap> loadThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass;
}
- if (threadContextMap == null) {
- return null;
- }
+ final String threadContextMap = getThreadContextMap();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || threadContextMap == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(threadContextMap);
if (ThreadContextMap.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(ThreadContextMap.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ threadContextMap,
+ getUrl(),
+ ThreadContextMap.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", threadContextMap, url.toString(), e);
+ LOGGER.error("Unable to load class {} specified in {}", threadContextMap, url.toString(), e);
}
return null;
}
+ /**
+ * Extension point for providers to create a {@link ThreadContextMap}
+ * <p>
+ * The default implementation:
+ * </p>
+ * <ol>
+ * <li>calls {@link #loadThreadContextMap},</li>
+ * <li>if the previous call returns {@code null}, it calls {@link #getThreadContextMap} to instantiate one of
+ * the internal implementations,</li>
+ * <li>it returns a no-op map otherwise.</li>
+ * </ol>
+ * @since 2.24.0
+ */
+ protected ThreadContextMap createThreadContextMap() {
Review Comment:
Similar to `createLoggerContextFactory()`, I don't understand why this needs to have `public` visibility.
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
Review Comment:
I have a feeling this is here only for tests. I cannot see it getting it used in a way that necessitates a `public` visibility. Am I missing something?
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java:
##########
@@ -43,40 +43,30 @@ public class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString
private final boolean useMap;
private final ThreadLocal<Map<String, String>> localMap;
- private static boolean inheritableMap;
-
- static {
- init();
- }
-
- // LOG4J2-479: by default, use a plain ThreadLocal, only use InheritableThreadLocal if configured.
- // (This method is package protected for JUnit tests.)
- static ThreadLocal<Map<String, String>> createThreadLocalMap(final boolean isMapEnabled) {
- if (inheritableMap) {
- return new InheritableThreadLocal<Map<String, String>>() {
- @Override
- protected Map<String, String> childValue(final Map<String, String> parentValue) {
- return parentValue != null && isMapEnabled //
- ? Collections.unmodifiableMap(new HashMap<>(parentValue)) //
- : null;
- }
- };
- }
- // if not inheritable, return plain ThreadLocal with null as initial value
- return new ThreadLocal<>();
- }
-
- static void init() {
- inheritableMap = PropertiesUtil.getProperties().getBooleanProperty(INHERITABLE_MAP);
- }
-
public DefaultThreadContextMap() {
this(true);
}
+ /**
+ * @deprecated Since 2.24.0 use the default constructor or {@link NoOpThreadContextMap} instead.
+ */
+ @Deprecated
public DefaultThreadContextMap(final boolean useMap) {
+ this(useMap, PropertiesUtil.getProperties());
+ }
+
+ DefaultThreadContextMap(final boolean useMap, final PropertiesUtil properties) {
this.useMap = useMap;
- this.localMap = createThreadLocalMap(useMap);
+ localMap = properties.getBooleanProperty(INHERITABLE_MAP)
+ ? new InheritableThreadLocal<Map<String, String>>() {
+ @Override
+ protected Map<String, String> childValue(final Map<String, String> parentValue) {
+ return parentValue != null && useMap
+ ? Collections.unmodifiableMap(new HashMap<>(parentValue))
+ : null;
+ }
+ }
+ : new ThreadLocal<Map<String, String>>();
Review Comment:
if `!useMap`, shouldn't `localMap` be simply set to `NoOpThreadContextMap`?
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
Review Comment:
Why do we seal this method with `final`? If I would have been a provider, I would rather only implement this method
1. don't deal with the ctor param
2. throw `UnsupportedOperationException` for `loadLoggerContextFactory()`
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -20,39 +20,135 @@
import java.net.URL;
import java.util.Properties;
import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.simple.SimpleLoggerContextFactory;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.Constants;
+import org.apache.logging.log4j.util.Lazy;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.apache.logging.log4j.util.PropertiesUtil;
/**
- * Model class for a Log4j 2 provider. The properties in this class correspond to the properties used in a
- * {@code META-INF/log4j-provider.properties} file. Note that this class is automatically created by Log4j and should
- * not be used by providers.
+ * Service class used to bind the Log4j API with an implementation.
+ * <p>
+ * Implementors should register an implementation of this class with {@link java.util.ServiceLoader}.
+ * </p>
+ * <p>
+ * <strong>Deprecated:</strong> the automatic registration of providers from
+ * {@code META-INF/log4j-provider.properties} is supported for compatibility reasons. Support for this file will
+ * be dropped in a future version.
+ * </p>
*/
public class Provider {
+ protected static final String CURRENT_VERSION = "2.6.0";
+
/**
* Property name to set for a Log4j 2 provider to specify the priority of this implementation.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String FACTORY_PRIORITY = "FactoryPriority";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.ThreadContextMap}.
+ * Property name to set to the implementation of {@link ThreadContextMap}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String THREAD_CONTEXT_MAP = "ThreadContextMap";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.LoggerContextFactory}.
+ * Property name to set to the implementation of {@link LoggerContextFactory}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String LOGGER_CONTEXT_FACTORY = "LoggerContextFactory";
- private static final Integer DEFAULT_PRIORITY = Integer.valueOf(-1);
+ /**
+ * System property used to specify the class name of the provider to use.
+ * @since 2.24.0
+ */
+ public static final String PROVIDER_PROPERTY_NAME = "log4j.provider";
+
+ // Bundled context map implementations
+ private static final String BASE = "org.apache.logging.log4j.internal.map.";
+
+ /**
+ * Constant used to disable the {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String NO_OP_CONTEXT_MAP = BASE + "NoOp";
+
+ /**
+ * Constant used to select a web application-safe implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation only binds JRE classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String WEB_APP_CONTEXT_MAP = BASE + "WebApp";
+
+ /**
+ * Constant used to select a copy-on-write implementation of {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String COPY_ON_WRITE_CONTEXT_MAP = BASE + "CopyOnWrite";
+
+ /**
+ * Constant used to select a garbage-free implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation must ensure that common operations don't create new object instances. The drawback is
+ * the necessity to bind custom classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String GARBAGE_FREE_CONTEXT_MAP = BASE + "GarbageFree";
Review Comment:
Do we need all these new `public` constants? Can we make it work if they would have been `private`? (Generally you are the adamant about avoiding to add new public fields... :thinking:)
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
Review Comment:
```suggestion
* @return a new logger context factory instance
```
_"A lazily initialized"_ statement is not correct, the returned instance is already initialized.
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
+ return loggerContextFactoryLazy.get();
+ }
+
+ /**
+ * Gets the class name of the {@link ThreadContextMap} implementation of this Provider.
+ * <p>
+ * This method should return one of the internal implementations:
+ * <ol>
+ * <li>{@code null} if {@link #loadThreadContextMap} is implemented,</li>
+ * <li>{@link #NO_OP_CONTEXT_MAP},</li>
+ * <li>{@link #WEB_APP_CONTEXT_MAP},</li>
+ * <li>{@link #COPY_ON_WRITE_CONTEXT_MAP},</li>
+ * <li>{@link #GARBAGE_FREE_CONTEXT_MAP}.</li>
+ * </ol>
+ * </p>
* @return the class name of a ThreadContextMap implementation
+ * @see #loadThreadContextMap()
*/
public String getThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass.getName();
}
- return threadContextMap;
+ // Field value
+ if (threadContextMap != null) {
+ return threadContextMap;
+ }
+ // Properties
+ final PropertiesUtil props = PropertiesUtil.getProperties();
+ if (props.getBooleanProperty(DISABLE_CONTEXT_MAP) || props.getBooleanProperty(DISABLE_THREAD_CONTEXT)) {
+ return NO_OP_CONTEXT_MAP;
+ }
+ final String threadContextMapClass = props.getStringProperty(THREAD_CONTEXT_MAP_PROPERTY);
+ if (threadContextMapClass != null) {
+ return threadContextMapClass;
+ }
+ // Default based on properties
+ if (Constants.ENABLE_THREADLOCALS) {
+ return props.getBooleanProperty(GC_FREE_THREAD_CONTEXT_PROPERTY)
+ ? GC_FREE_THREAD_CONTEXT_PROPERTY
+ : COPY_ON_WRITE_CONTEXT_MAP;
+ }
+ return WEB_APP_CONTEXT_MAP;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.ThreadContextMap} class specified by this Provider.
+ * Loads the {@link ThreadContextMap} class specified by this Provider.
*
- * @return the ThreadContextMap implementation class or {@code null} if there was an error loading it
+ * @return the {@code ThreadContextMap} implementation class or {@code null} if unspecified or a loading error
+ * occurred.
+ * @see #createThreadContextMap()
*/
public Class<? extends ThreadContextMap> loadThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass;
}
- if (threadContextMap == null) {
- return null;
- }
+ final String threadContextMap = getThreadContextMap();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || threadContextMap == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(threadContextMap);
if (ThreadContextMap.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(ThreadContextMap.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ threadContextMap,
+ getUrl(),
+ ThreadContextMap.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", threadContextMap, url.toString(), e);
+ LOGGER.error("Unable to load class {} specified in {}", threadContextMap, url.toString(), e);
}
return null;
}
+ /**
+ * Extension point for providers to create a {@link ThreadContextMap}
+ * <p>
+ * The default implementation:
+ * </p>
+ * <ol>
+ * <li>calls {@link #loadThreadContextMap},</li>
+ * <li>if the previous call returns {@code null}, it calls {@link #getThreadContextMap} to instantiate one of
+ * the internal implementations,</li>
+ * <li>it returns a no-op map otherwise.</li>
+ * </ol>
+ * @since 2.24.0
+ */
+ protected ThreadContextMap createThreadContextMap() {
+ final Class<? extends ThreadContextMap> threadContextMapClass = loadThreadContextMap();
+ if (threadContextMapClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(threadContextMapClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}",
+ threadContextMapClass.getName(),
+ getUrl(),
+ e);
+ }
+ }
+ // Standard Log4j API implementations are internal and can be only specified by name:
+ final String threadContextMap = getThreadContextMap();
+ if (threadContextMap != null) {
+ /*
+ * The constructors are called explicitly to improve GraalVM support.
+ *
+ * The class names of the package-private implementations from version 2.23.1 must be recognized even
+ * if the class is moved.
+ */
+ switch (threadContextMap) {
+ case NO_OP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.NoOpThreadContextMap":
+ return new NoOpThreadContextMap();
+ case WEB_APP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.DefaultThreadContextMap":
+ return new DefaultThreadContextMap();
+ case GARBAGE_FREE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.GarbageFreeSortedArrayThreadContextMap":
+ return new GarbageFreeSortedArrayThreadContextMap();
+ case COPY_ON_WRITE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.CopyOnWriteSortedArrayThreadContextMap":
+ return new CopyOnWriteSortedArrayThreadContextMap();
+ }
+ }
+ LOGGER.warn("Falling back to {}", NoOpThreadContextMap.class.getName());
+ return new NoOpThreadContextMap();
+ }
+
+ // Used for testing
+ void resetThreadContextMap() {
+ threadContextMapLazy.set(null);
+ }
+
+ /**
+ * @return A lazily initialized thread context map.
Review Comment:
```suggestion
* @return a new thread context map instance
```
_"A lazily initialized"_ statement is not correct, the returned instance is already initialized.
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
+ return loggerContextFactoryLazy.get();
+ }
+
+ /**
+ * Gets the class name of the {@link ThreadContextMap} implementation of this Provider.
+ * <p>
+ * This method should return one of the internal implementations:
+ * <ol>
+ * <li>{@code null} if {@link #loadThreadContextMap} is implemented,</li>
+ * <li>{@link #NO_OP_CONTEXT_MAP},</li>
+ * <li>{@link #WEB_APP_CONTEXT_MAP},</li>
+ * <li>{@link #COPY_ON_WRITE_CONTEXT_MAP},</li>
+ * <li>{@link #GARBAGE_FREE_CONTEXT_MAP}.</li>
+ * </ol>
+ * </p>
* @return the class name of a ThreadContextMap implementation
+ * @see #loadThreadContextMap()
*/
public String getThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass.getName();
}
- return threadContextMap;
+ // Field value
+ if (threadContextMap != null) {
+ return threadContextMap;
+ }
+ // Properties
+ final PropertiesUtil props = PropertiesUtil.getProperties();
+ if (props.getBooleanProperty(DISABLE_CONTEXT_MAP) || props.getBooleanProperty(DISABLE_THREAD_CONTEXT)) {
+ return NO_OP_CONTEXT_MAP;
+ }
+ final String threadContextMapClass = props.getStringProperty(THREAD_CONTEXT_MAP_PROPERTY);
+ if (threadContextMapClass != null) {
+ return threadContextMapClass;
+ }
+ // Default based on properties
+ if (Constants.ENABLE_THREADLOCALS) {
+ return props.getBooleanProperty(GC_FREE_THREAD_CONTEXT_PROPERTY)
+ ? GC_FREE_THREAD_CONTEXT_PROPERTY
+ : COPY_ON_WRITE_CONTEXT_MAP;
+ }
+ return WEB_APP_CONTEXT_MAP;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.ThreadContextMap} class specified by this Provider.
+ * Loads the {@link ThreadContextMap} class specified by this Provider.
*
- * @return the ThreadContextMap implementation class or {@code null} if there was an error loading it
+ * @return the {@code ThreadContextMap} implementation class or {@code null} if unspecified or a loading error
+ * occurred.
+ * @see #createThreadContextMap()
*/
public Class<? extends ThreadContextMap> loadThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass;
}
- if (threadContextMap == null) {
- return null;
- }
+ final String threadContextMap = getThreadContextMap();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || threadContextMap == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(threadContextMap);
if (ThreadContextMap.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(ThreadContextMap.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ threadContextMap,
+ getUrl(),
+ ThreadContextMap.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", threadContextMap, url.toString(), e);
+ LOGGER.error("Unable to load class {} specified in {}", threadContextMap, url.toString(), e);
}
return null;
}
+ /**
+ * Extension point for providers to create a {@link ThreadContextMap}
+ * <p>
+ * The default implementation:
+ * </p>
+ * <ol>
+ * <li>calls {@link #loadThreadContextMap},</li>
+ * <li>if the previous call returns {@code null}, it calls {@link #getThreadContextMap} to instantiate one of
+ * the internal implementations,</li>
+ * <li>it returns a no-op map otherwise.</li>
+ * </ol>
+ * @since 2.24.0
+ */
+ protected ThreadContextMap createThreadContextMap() {
+ final Class<? extends ThreadContextMap> threadContextMapClass = loadThreadContextMap();
+ if (threadContextMapClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(threadContextMapClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}",
+ threadContextMapClass.getName(),
+ getUrl(),
+ e);
+ }
+ }
+ // Standard Log4j API implementations are internal and can be only specified by name:
+ final String threadContextMap = getThreadContextMap();
+ if (threadContextMap != null) {
+ /*
+ * The constructors are called explicitly to improve GraalVM support.
+ *
+ * The class names of the package-private implementations from version 2.23.1 must be recognized even
+ * if the class is moved.
+ */
+ switch (threadContextMap) {
+ case NO_OP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.NoOpThreadContextMap":
+ return new NoOpThreadContextMap();
+ case WEB_APP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.DefaultThreadContextMap":
+ return new DefaultThreadContextMap();
+ case GARBAGE_FREE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.GarbageFreeSortedArrayThreadContextMap":
+ return new GarbageFreeSortedArrayThreadContextMap();
+ case COPY_ON_WRITE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.CopyOnWriteSortedArrayThreadContextMap":
+ return new CopyOnWriteSortedArrayThreadContextMap();
+ }
+ }
+ LOGGER.warn("Falling back to {}", NoOpThreadContextMap.class.getName());
+ return new NoOpThreadContextMap();
+ }
+
+ // Used for testing
+ void resetThreadContextMap() {
+ threadContextMapLazy.set(null);
+ }
+
+ /**
+ * @return A lazily initialized thread context map.
+ * @since 2.24.0
+ */
+ public final ThreadContextMap getThreadContextMapInstance() {
Review Comment:
Similar to `getLoggerContextFactory()`, I don't understand why this is `final`.
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -97,127 +217,266 @@ public String getVersions() {
/**
* Gets the priority (natural ordering) of this Provider.
- *
+ * <p>
+ * Log4j selects the highest priority provider.
+ * </p>
* @return the priority of this Provider
*/
public Integer getPriority() {
return priority;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation of this
- * Provider.
+ * Gets the class name of the {@link LoggerContextFactory} implementation of this Provider.
*
- * @return the class name of a LoggerContextFactory implementation
+ * @return the class name of a LoggerContextFactory implementation or {@code null} if unspecified.
+ * @see #loadLoggerContextFactory()
*/
public String getClassName() {
- if (loggerContextFactoryClass != null) {
- return loggerContextFactoryClass.getName();
- }
- return className;
+ return loggerContextFactoryClass != null ? loggerContextFactoryClass.getName() : className;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.LoggerContextFactory} class specified by this Provider.
+ * Loads the {@link LoggerContextFactory} class specified by this Provider.
*
- * @return the LoggerContextFactory implementation class or {@code null} if there was an error loading it
+ * @return the LoggerContextFactory implementation class or {@code null} if unspecified or a loader error occurred.
+ * @see #createLoggerContextFactory()
*/
public Class<? extends LoggerContextFactory> loadLoggerContextFactory() {
if (loggerContextFactoryClass != null) {
return loggerContextFactoryClass;
}
- if (className == null) {
- return null;
- }
+ final String className = getClassName();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || className == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(className);
if (LoggerContextFactory.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(LoggerContextFactory.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ className,
+ getUrl(),
+ LoggerContextFactory.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", className, url.toString(), e);
+ LOGGER.error("Unable to create class {} specified in {}", className, getUrl(), e);
}
return null;
}
/**
- * Gets the class name of the {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation of this Provider.
- *
+ * Extension point for providers to create a {@link LoggerContextFactory}.
+ * @since 2.24.0
+ */
+ protected LoggerContextFactory createLoggerContextFactory() {
+ final Class<? extends LoggerContextFactory> factoryClass = loadLoggerContextFactory();
+ if (factoryClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(factoryClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}", factoryClass.getName(), getUrl(), e);
+ }
+ }
+ LOGGER.warn("Falling back to {}", SimpleLoggerContextFactory.INSTANCE);
+ return SimpleLoggerContextFactory.INSTANCE;
+ }
+
+ /**
+ * @return A lazily initialized logger context factory
+ * @since 2.24.0
+ */
+ public final LoggerContextFactory getLoggerContextFactory() {
+ return loggerContextFactoryLazy.get();
+ }
+
+ /**
+ * Gets the class name of the {@link ThreadContextMap} implementation of this Provider.
+ * <p>
+ * This method should return one of the internal implementations:
+ * <ol>
+ * <li>{@code null} if {@link #loadThreadContextMap} is implemented,</li>
+ * <li>{@link #NO_OP_CONTEXT_MAP},</li>
+ * <li>{@link #WEB_APP_CONTEXT_MAP},</li>
+ * <li>{@link #COPY_ON_WRITE_CONTEXT_MAP},</li>
+ * <li>{@link #GARBAGE_FREE_CONTEXT_MAP}.</li>
+ * </ol>
+ * </p>
* @return the class name of a ThreadContextMap implementation
+ * @see #loadThreadContextMap()
*/
public String getThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass.getName();
}
- return threadContextMap;
+ // Field value
+ if (threadContextMap != null) {
+ return threadContextMap;
+ }
+ // Properties
+ final PropertiesUtil props = PropertiesUtil.getProperties();
+ if (props.getBooleanProperty(DISABLE_CONTEXT_MAP) || props.getBooleanProperty(DISABLE_THREAD_CONTEXT)) {
+ return NO_OP_CONTEXT_MAP;
+ }
+ final String threadContextMapClass = props.getStringProperty(THREAD_CONTEXT_MAP_PROPERTY);
+ if (threadContextMapClass != null) {
+ return threadContextMapClass;
+ }
+ // Default based on properties
+ if (Constants.ENABLE_THREADLOCALS) {
+ return props.getBooleanProperty(GC_FREE_THREAD_CONTEXT_PROPERTY)
+ ? GC_FREE_THREAD_CONTEXT_PROPERTY
+ : COPY_ON_WRITE_CONTEXT_MAP;
+ }
+ return WEB_APP_CONTEXT_MAP;
}
/**
- * Loads the {@link org.apache.logging.log4j.spi.ThreadContextMap} class specified by this Provider.
+ * Loads the {@link ThreadContextMap} class specified by this Provider.
*
- * @return the ThreadContextMap implementation class or {@code null} if there was an error loading it
+ * @return the {@code ThreadContextMap} implementation class or {@code null} if unspecified or a loading error
+ * occurred.
+ * @see #createThreadContextMap()
*/
public Class<? extends ThreadContextMap> loadThreadContextMap() {
if (threadContextMapClass != null) {
return threadContextMapClass;
}
- if (threadContextMap == null) {
- return null;
- }
+ final String threadContextMap = getThreadContextMap();
final ClassLoader loader = classLoader.get();
- if (loader == null) {
+ // Support for deprecated {@code META-INF/log4j-provider.properties} format.
+ // In the remaining cases {@code loader == null}.
+ if (loader == null || threadContextMap == null) {
return null;
}
try {
final Class<?> clazz = loader.loadClass(threadContextMap);
if (ThreadContextMap.class.isAssignableFrom(clazz)) {
return clazz.asSubclass(ThreadContextMap.class);
+ } else {
+ LOGGER.error(
+ "Class {} specified in {} does not extend {}",
+ threadContextMap,
+ getUrl(),
+ ThreadContextMap.class.getName());
}
} catch (final Exception e) {
- LOGGER.error("Unable to create class {} specified in {}", threadContextMap, url.toString(), e);
+ LOGGER.error("Unable to load class {} specified in {}", threadContextMap, url.toString(), e);
}
return null;
}
+ /**
+ * Extension point for providers to create a {@link ThreadContextMap}
+ * <p>
+ * The default implementation:
+ * </p>
+ * <ol>
+ * <li>calls {@link #loadThreadContextMap},</li>
+ * <li>if the previous call returns {@code null}, it calls {@link #getThreadContextMap} to instantiate one of
+ * the internal implementations,</li>
+ * <li>it returns a no-op map otherwise.</li>
+ * </ol>
+ * @since 2.24.0
+ */
+ protected ThreadContextMap createThreadContextMap() {
+ final Class<? extends ThreadContextMap> threadContextMapClass = loadThreadContextMap();
+ if (threadContextMapClass != null) {
+ try {
+ return LoaderUtil.newInstanceOf(threadContextMapClass);
+ } catch (final Exception e) {
+ LOGGER.error(
+ "Unable to create instance of class {} specified in {}",
+ threadContextMapClass.getName(),
+ getUrl(),
+ e);
+ }
+ }
+ // Standard Log4j API implementations are internal and can be only specified by name:
+ final String threadContextMap = getThreadContextMap();
+ if (threadContextMap != null) {
+ /*
+ * The constructors are called explicitly to improve GraalVM support.
+ *
+ * The class names of the package-private implementations from version 2.23.1 must be recognized even
+ * if the class is moved.
+ */
+ switch (threadContextMap) {
+ case NO_OP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.NoOpThreadContextMap":
+ return new NoOpThreadContextMap();
+ case WEB_APP_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.DefaultThreadContextMap":
+ return new DefaultThreadContextMap();
+ case GARBAGE_FREE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.GarbageFreeSortedArrayThreadContextMap":
+ return new GarbageFreeSortedArrayThreadContextMap();
+ case COPY_ON_WRITE_CONTEXT_MAP:
+ case "org.apache.logging.log4j.spi.CopyOnWriteSortedArrayThreadContextMap":
+ return new CopyOnWriteSortedArrayThreadContextMap();
+ }
+ }
+ LOGGER.warn("Falling back to {}", NoOpThreadContextMap.class.getName());
+ return new NoOpThreadContextMap();
+ }
+
+ // Used for testing
+ void resetThreadContextMap() {
+ threadContextMapLazy.set(null);
+ }
+
+ /**
+ * @return A lazily initialized thread context map.
+ * @since 2.24.0
+ */
+ public final ThreadContextMap getThreadContextMapInstance() {
+ return threadContextMapLazy.get();
+ }
+
/**
* Gets the URL containing this Provider's Log4j details.
*
- * @return the URL corresponding to the Provider {@code META-INF/log4j-provider.properties} file
+ * @return the URL corresponding to the Provider {@code META-INF/log4j-provider.properties} file or {@code null}
+ * for a provider class.
+ * @deprecated since 2.24.0, without replacement.
*/
+ @Deprecated
public URL getUrl() {
return url;
}
@Override
public String toString() {
- final StringBuilder result = new StringBuilder("Provider[");
+ final StringBuilder result =
+ new StringBuilder("Provider '").append(getClass().getName()).append("'");
if (!DEFAULT_PRIORITY.equals(priority)) {
- result.append("priority=").append(priority).append(", ");
+ result.append("\n\tpriority = ").append(priority);
}
+ final String threadContextMap = getThreadContextMap();
if (threadContextMap != null) {
- result.append("threadContextMap=").append(threadContextMap).append(", ");
- } else if (threadContextMapClass != null) {
- result.append("threadContextMapClass=").append(threadContextMapClass.getName());
+ result.append("\n\tthreadContextMap = ").append(threadContextMap);
}
- if (className != null) {
- result.append("className=").append(className).append(", ");
- } else if (loggerContextFactoryClass != null) {
- result.append("class=").append(loggerContextFactoryClass.getName());
+ final String loggerContextFactory = getClassName();
+ if (loggerContextFactory != null) {
+ result.append("\n\tloggerContextFactory = ").append(loggerContextFactory);
}
if (url != null) {
- result.append("url=").append(url);
+ result.append("\n\turl = ").append(url);
}
- final ClassLoader loader;
- if (classLoader == null || (loader = classLoader.get()) == null) {
- result.append(", classLoader=null(not reachable)");
- } else {
- result.append(", classLoader=").append(loader);
+ if (Provider.class.equals(getClass())) {
+ final ClassLoader loader = classLoader.get();
+ if (loader == null) {
+ result.append("\n\tclassLoader = null or not reachable");
+ } else {
+ result.append("\n\tclassLoader = ").append(loader);
Review Comment:
Please don't do this type of indentation. It is a matter of time somebody will screw up the alignment. I prefer the earlier version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1527192000
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########
@@ -20,39 +20,135 @@
import java.net.URL;
import java.util.Properties;
import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.simple.SimpleLoggerContextFactory;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.Constants;
+import org.apache.logging.log4j.util.Lazy;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.apache.logging.log4j.util.PropertiesUtil;
/**
- * Model class for a Log4j 2 provider. The properties in this class correspond to the properties used in a
- * {@code META-INF/log4j-provider.properties} file. Note that this class is automatically created by Log4j and should
- * not be used by providers.
+ * Service class used to bind the Log4j API with an implementation.
+ * <p>
+ * Implementors should register an implementation of this class with {@link java.util.ServiceLoader}.
+ * </p>
+ * <p>
+ * <strong>Deprecated:</strong> the automatic registration of providers from
+ * {@code META-INF/log4j-provider.properties} is supported for compatibility reasons. Support for this file will
+ * be dropped in a future version.
+ * </p>
*/
public class Provider {
+ protected static final String CURRENT_VERSION = "2.6.0";
+
/**
* Property name to set for a Log4j 2 provider to specify the priority of this implementation.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String FACTORY_PRIORITY = "FactoryPriority";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.ThreadContextMap}.
+ * Property name to set to the implementation of {@link ThreadContextMap}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String THREAD_CONTEXT_MAP = "ThreadContextMap";
+
/**
- * Property name to set to the implementation of {@link org.apache.logging.log4j.spi.LoggerContextFactory}.
+ * Property name to set to the implementation of {@link LoggerContextFactory}.
+ * @deprecated since 2.24.0
*/
+ @Deprecated
public static final String LOGGER_CONTEXT_FACTORY = "LoggerContextFactory";
- private static final Integer DEFAULT_PRIORITY = Integer.valueOf(-1);
+ /**
+ * System property used to specify the class name of the provider to use.
+ * @since 2.24.0
+ */
+ public static final String PROVIDER_PROPERTY_NAME = "log4j.provider";
+
+ // Bundled context map implementations
+ private static final String BASE = "org.apache.logging.log4j.internal.map.";
+
+ /**
+ * Constant used to disable the {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String NO_OP_CONTEXT_MAP = BASE + "NoOp";
+
+ /**
+ * Constant used to select a web application-safe implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation only binds JRE classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String WEB_APP_CONTEXT_MAP = BASE + "WebApp";
+
+ /**
+ * Constant used to select a copy-on-write implementation of {@link ThreadContextMap}.
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String COPY_ON_WRITE_CONTEXT_MAP = BASE + "CopyOnWrite";
+
+ /**
+ * Constant used to select a garbage-free implementation of {@link ThreadContextMap}.
+ * <p>
+ * This implementation must ensure that common operations don't create new object instances. The drawback is
+ * the necessity to bind custom classes to {@link ThreadLocal} variables.
+ * </p>
+ * <p>
+ * <strong>Warning:</strong> the value of this constant does not point to a concrete class name.
+ * </p>
+ * @see #getThreadContextMap
+ */
+ public static final String GARBAGE_FREE_CONTEXT_MAP = BASE + "GarbageFree";
Review Comment:
You are right, they could be `protected`.
The reason I introduced these constants is:
- half of these implementations are package-private. An implementation of `Provider` can not instantiate them, but `Provider` itself can. To use these context maps an implementation should:
1. implement `#getThreadContextMap` and return one of these special constants,
2. make a call to `super.createThreadContextMap()` in its `#createThreadContextMap` implementation.
- I wanted to separate the characteristics of the context map, from the implementation. So the constant `WebApp` can switch to @jengebr's implementation under the hood. Probably I should make these constants shorter.
BTW: IMHO the inclusion of `ThreadContextMap` implementations in the `log4j-api` unnecessarily bloats the artifact. `ThreadContext` is not a standalone service for users to store thread-dependent data (although I saw some users use it for this purpose). It must be tightly linked to the `Logger` implementation. As such, except Log4j Core, no other Log4j API implementation requires these 3 implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#discussion_r1527196204
##########
log4j-api/src/main/java/org/apache/logging/log4j/util/ProviderUtil.java:
##########
@@ -163,6 +172,88 @@ static void lazyInit() {
}
}
+ /**
+ * Used to test the public {@link #getProvider()} method.
+ */
+ static Provider selectProvider(final PropertiesUtil properties) {
Review Comment:
I tested the externally observable behavior:
1. the `log4j.provider` system property creates a provider with the specified class name,
2. the `log4j2.loggerContextFactory` system property create a provider with the specified logger context factory class,
3. if more than one implementation is available, the implementation with highest `getPriority` wins.
What happens in the remaining cases (multiple system properties set, typos in the names of properties) are in my opinion implementation details and we can change them at will without breaking the `#getProvider()` contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Centralize initialization in `Provider` class (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #2374:
URL: https://github.com/apache/logging-log4j2/pull/2374#issuecomment-2174304317
Those internal/private classes are hardwired in `Provider`, since implementations of `Provider` have no way to access those classes.
Once I'll promote the `StringArrayThreadContextMap` to default thread context map, I'll be able to either:
* remove those private implementations (since the default context map can be used for garbage-free logging)
* or move them to `log4j-core` (since no other implementation really profits from them).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org