From 9a70d3a41b7bb9993b80d155bab160b72568f63f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 9 Sep 2024 22:13:32 +0200 Subject: [PATCH 01/17] Deprecate `AbstractLogger.checkMessageFactory()` --- .../apache/logging/log4j/simple/SimpleLoggerContext.java | 1 - .../java/org/apache/logging/log4j/spi/AbstractLogger.java | 3 +++ .../java/org/apache/logging/log4j/core/LoggerContext.java | 1 - .../logging/log4j/taglib/Log4jTaglibLoggerContext.java | 1 - .../deprecate_AbstractLogger_checkMessageFactory.xml | 8 ++++++++ 5 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java index d1aaf8db0f7..7ac6871bb89 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java @@ -100,7 +100,6 @@ public ExtendedLogger getLogger(final String name, final MessageFactory messageF // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. final ExtendedLogger extendedLogger = loggerRegistry.getLogger(name, messageFactory); if (extendedLogger != null) { - AbstractLogger.checkMessageFactory(extendedLogger, messageFactory); return extendedLogger; } final SimpleLogger simpleLogger = new SimpleLogger( diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java index ecc499ac586..5011900fd2c 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java @@ -166,7 +166,10 @@ private static MessageFactory2 adaptMessageFactory(final MessageFactory result) * * @param logger The logger to check * @param messageFactory The message factory to check. + * @deprecated Planned to be removed! + * Instead, in {@link LoggerContext#getLogger(String, MessageFactory)} implementations, namespace loggers with message factories. */ + @Deprecated public static void checkMessageFactory(final ExtendedLogger logger, final MessageFactory messageFactory) { final String name = logger.getName(); final MessageFactory loggerMessageFactory = logger.getMessageFactory(); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 07cd2d1f592..674c4ef98cb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -527,7 +527,6 @@ public Logger getLogger(final String name, final MessageFactory messageFactory) // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. Logger logger = loggerRegistry.getLogger(name, messageFactory); if (logger != null) { - AbstractLogger.checkMessageFactory(logger, messageFactory); return logger; } diff --git a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java index 3c76f39b21a..6253659811c 100644 --- a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java +++ b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java @@ -60,7 +60,6 @@ public Log4jTaglibLogger getLogger(final String name, final MessageFactory messa // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. Log4jTaglibLogger logger = this.loggerRegistry.getLogger(name, messageFactory); if (logger != null) { - AbstractLogger.checkMessageFactory(logger, messageFactory); return logger; } diff --git a/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml b/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml new file mode 100644 index 00000000000..62cbc2d8372 --- /dev/null +++ b/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml @@ -0,0 +1,8 @@ + + + + Deprecate `AbstractLogger.checkMessageFactory()`, since all created `Logger`s are already `MessageFactory`-namespaced + From 55eb07bcadb9ceca9c96664d22b646742ab6ad8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 9 Sep 2024 22:14:42 +0200 Subject: [PATCH 02/17] Fix changelog entry --- .../.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml b/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml index 62cbc2d8372..e72a33566a4 100644 --- a/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml +++ b/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml @@ -3,6 +3,6 @@ xmlns="https://logging.apache.org/xml/ns" xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" type="deprecated"> - + Deprecate `AbstractLogger.checkMessageFactory()`, since all created `Logger`s are already `MessageFactory`-namespaced From af527a006d18698b68e6ad9693cbf7b0a04a1ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 9 Sep 2024 22:21:40 +0200 Subject: [PATCH 03/17] Fix Spotless failures --- .../org/apache/logging/log4j/simple/SimpleLoggerContext.java | 1 - .../main/java/org/apache/logging/log4j/core/LoggerContext.java | 1 - .../apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java | 1 - 3 files changed, 3 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java index 7ac6871bb89..48ba50a0575 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java @@ -21,7 +21,6 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.simple.internal.SimpleProvider; -import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.LoggerRegistry; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 674c4ef98cb..3aa72f0d169 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -48,7 +48,6 @@ import org.apache.logging.log4j.core.util.NetUtils; import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry; import org.apache.logging.log4j.message.MessageFactory; -import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.LoggerContextFactory; import org.apache.logging.log4j.spi.LoggerContextShutdownAware; import org.apache.logging.log4j.spi.LoggerContextShutdownEnabled; diff --git a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java index 6253659811c..1a38afd578c 100644 --- a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java +++ b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java @@ -20,7 +20,6 @@ import javax.servlet.ServletContext; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.MessageFactory; -import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.LoggerRegistry; From be6c229307fb907b1c70fbb421c3a8683907452e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 11:40:46 +0200 Subject: [PATCH 04/17] Implement `MessageFactory`-namespaced logger registry --- .../log4j/simple/SimpleLoggerContext.java | 31 +- .../logging/log4j/simple/package-info.java | 2 +- .../logging/log4j/spi/AbstractLogger.java | 3 +- .../logging/log4j/spi/LoggerContext.java | 7 +- .../logging/log4j/spi/LoggerRegistry.java | 284 ++++++++++++++---- .../logging/log4j/spi/package-info.java | 2 +- .../logging/log4j/core/LoggerContext.java | 54 ++-- .../log4j/core/async/package-info.java | 2 +- .../logging/log4j/core/package-info.java | 2 +- log4j-taglib/pom.xml | 12 +- .../taglib/Log4jTaglibLoggerContext.java | 90 +++--- .../logging/log4j/taglib/package-info.java | 2 +- .../logging/log4j/tojul/JULLoggerContext.java | 33 +- .../logging/log4j/tojul/package-info.java | 2 +- .../logging/slf4j/SLF4JLoggerContext.java | 34 ++- .../apache/logging/slf4j/package-info.java | 2 +- 16 files changed, 389 insertions(+), 173 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java index 48ba50a0575..e4052e87a48 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java @@ -20,11 +20,13 @@ import java.io.PrintStream; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.simple.internal.SimpleProvider; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.LoggerRegistry; import org.apache.logging.log4j.util.PropertiesUtil; +import org.jspecify.annotations.Nullable; /** * A simple {@link LoggerContext} implementation. @@ -40,6 +42,8 @@ public class SimpleLoggerContext implements LoggerContext { /** All system properties used by SimpleLog start with this */ protected static final String SYSTEM_PREFIX = "org.apache.logging.log4j.simplelog."; + private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE; + private final PropertiesUtil props; /** Include the instance name in the log message? */ @@ -95,13 +99,14 @@ public ExtendedLogger getLogger(final String name) { } @Override - public ExtendedLogger getLogger(final String name, final MessageFactory messageFactory) { - // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. - final ExtendedLogger extendedLogger = loggerRegistry.getLogger(name, messageFactory); - if (extendedLogger != null) { - return extendedLogger; - } - final SimpleLogger simpleLogger = new SimpleLogger( + public ExtendedLogger getLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::createLogger); + } + + private ExtendedLogger createLogger(final String name, @Nullable final MessageFactory messageFactory) { + return new SimpleLogger( name, defaultLevel, showLogName, @@ -112,8 +117,6 @@ public ExtendedLogger getLogger(final String name, final MessageFactory messageF messageFactory, props, stream); - loggerRegistry.putIfAbsent(name, messageFactory, simpleLogger); - return loggerRegistry.getLogger(name, messageFactory); } /** @@ -129,16 +132,18 @@ public LoggerRegistry getLoggerRegistry() { @Override public boolean hasLogger(final String name) { - return false; + return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY); } @Override public boolean hasLogger(final String name, final Class messageFactoryClass) { - return false; + return loggerRegistry.hasLogger(name, messageFactoryClass); } @Override - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return false; + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java index 1481df9462e..29139a12ace 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java @@ -20,7 +20,7 @@ * Providers are able to be loaded at runtime. */ @Export -@Version("2.24.0") +@Version("2.24.1") package org.apache.logging.log4j.simple; import org.osgi.annotation.bundle.Export; diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java index 5011900fd2c..a92a4cd06d3 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java @@ -166,8 +166,9 @@ private static MessageFactory2 adaptMessageFactory(final MessageFactory result) * * @param logger The logger to check * @param messageFactory The message factory to check. - * @deprecated Planned to be removed! + * @deprecated As of version {@code 2.24.1}, planned to be removed! * Instead, in {@link LoggerContext#getLogger(String, MessageFactory)} implementations, namespace loggers with message factories. + * If your implementation uses {@link LoggerRegistry}, you are already covered. */ @Deprecated public static void checkMessageFactory(final ExtendedLogger logger, final MessageFactory messageFactory) { diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java index a14af967bf4..27874bded06 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java @@ -18,6 +18,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.MessageFactory; +import org.jspecify.annotations.Nullable; /** * Anchor point for logging implementations. @@ -54,7 +55,7 @@ default ExtendedLogger getLogger(Class cls) { * @return The logger. * @since 2.14.0 */ - default ExtendedLogger getLogger(Class cls, MessageFactory messageFactory) { + default ExtendedLogger getLogger(Class cls, @Nullable MessageFactory messageFactory) { final String canonicalName = cls.getCanonicalName(); return getLogger(canonicalName != null ? canonicalName : cls.getName(), messageFactory); } @@ -73,7 +74,7 @@ default ExtendedLogger getLogger(Class cls, MessageFactory messageFactory) { * the logger but will log a warning if mismatched. * @return The logger with the specified name. */ - ExtendedLogger getLogger(String name, MessageFactory messageFactory); + ExtendedLogger getLogger(String name, @Nullable MessageFactory messageFactory); /** * Gets the LoggerRegistry. @@ -118,7 +119,7 @@ default Object getObject(String key) { * @return true if the Logger exists, false otherwise. * @since 2.5 */ - boolean hasLogger(String name, MessageFactory messageFactory); + boolean hasLogger(String name, @Nullable MessageFactory messageFactory); /** * Associates an object into the LoggerContext by name for later use. diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java index e71c0a933db..eac316fc448 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java @@ -16,28 +16,47 @@ */ package org.apache.logging.log4j.spi; +import static java.util.Objects.requireNonNull; + import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.status.StatusLogger; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** - * Convenience class to be used by {@code LoggerContext} implementations. + * Convenience class to be used as an {@link ExtendedLogger} registry by {@code LoggerContext} implementations. */ +@NullMarked public class LoggerRegistry { - private static final String DEFAULT_FACTORY_KEY = AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS.getName(); - private final MapFactory factory; - private final Map> map; + + private final Map> loggerByMessageFactoryByName = new HashMap<>(); + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + private final Lock readLock = lock.readLock(); + + private final Lock writeLock = lock.writeLock(); /** - * Interface to control the data structure used by the registry to store the Loggers. + * Data structure contract for the internal storage of admitted loggers. + * * @param subtype of {@code ExtendedLogger} + * @deprecated As of version {@code 2.24.1}, planned to be removed! */ + @Deprecated public interface MapFactory { + Map createInnerMap(); Map> createOuterMap(); @@ -46,10 +65,14 @@ public interface MapFactory { } /** - * Generates ConcurrentHashMaps for use by the registry to store the Loggers. + * {@link MapFactory} implementation using {@link ConcurrentHashMap}. + * * @param subtype of {@code ExtendedLogger} + * @deprecated As of version {@code 2.24.1}, planned to be removed! */ + @Deprecated public static class ConcurrentMapFactory implements MapFactory { + @Override public Map createInnerMap() { return new ConcurrentHashMap<>(); @@ -62,15 +85,19 @@ public Map> createOuterMap() { @Override public void putIfAbsent(final Map innerMap, final String name, final T logger) { - ((ConcurrentMap) innerMap).putIfAbsent(name, logger); + innerMap.putIfAbsent(name, logger); } } /** - * Generates WeakHashMaps for use by the registry to store the Loggers. + * {@link MapFactory} implementation using {@link WeakHashMap}. + * * @param subtype of {@code ExtendedLogger} + * @deprecated As of version {@code 2.24.1}, planned to be removed! */ + @Deprecated public static class WeakMapFactory implements MapFactory { + @Override public Map createInnerMap() { return new WeakHashMap<>(); @@ -87,43 +114,66 @@ public void putIfAbsent(final Map innerMap, final String name, final } } - public LoggerRegistry() { - this(new ConcurrentMapFactory()); - } + public LoggerRegistry() {} - public LoggerRegistry(final MapFactory factory) { - this.factory = Objects.requireNonNull(factory, "factory"); - this.map = factory.createOuterMap(); - } - - private static String factoryClassKey(final Class messageFactoryClass) { - return messageFactoryClass == null ? DEFAULT_FACTORY_KEY : messageFactoryClass.getName(); - } - - private static String factoryKey(final MessageFactory messageFactory) { - return messageFactory == null - ? DEFAULT_FACTORY_KEY - : messageFactory.getClass().getName(); + /** + * Constructs an instance ignoring the given the map factory. + * + * @param mapFactory a map factory + * @deprecated As of version {@code 2.24.1}, planned to be removed! + */ + @Deprecated + public LoggerRegistry(@Nullable final MapFactory mapFactory) { + this(); } /** - * Returns an ExtendedLogger. - * @param name The name of the Logger to return. - * @return The logger with the specified name. + * Returns the logger associated with the given name. + *

+ * There can be made no assumptions on the message factory of the returned logger. + * Callers are strongly advised to switch to {@link #getLogger(String, MessageFactory)} and provide a message factory parameter! + *

+ * + * @param name a logger name + * @return the logger associated with the name + * @deprecated As of version {@code 2.24.1}, planned to be removed! + * Use {@link #getLogger(String, MessageFactory)} instead. */ + @Deprecated public T getLogger(final String name) { - return getOrCreateInnerMap(DEFAULT_FACTORY_KEY).get(name); + requireNonNull(name, "name"); + return getLogger(name, null); } /** - * Returns an ExtendedLogger. - * @param name The name of the Logger to return. - * @param messageFactory The message factory is used only when creating a logger, subsequent use does not change - * the logger but will log a warning if mismatched. - * @return The logger with the specified name. + * Returns the logger associated with the given name and message factory. + *

+ * In the absence of a message factory, there can be made no assumptions on the message factory of the returned logger. + * This lenient behaviour is only kept for backward compatibility. + * Callers are strongly advised to provide a message factory parameter to the method! + *

+ * + * @param name a logger name + * @param messageFactory a message factory + * @return the logger associated with the given name and message factory */ - public T getLogger(final String name, final MessageFactory messageFactory) { - return getOrCreateInnerMap(factoryKey(messageFactory)).get(name); + public T getLogger(final String name, @Nullable final MessageFactory messageFactory) { + requireNonNull(name, "name"); + readLock.lock(); + try { + final Map loggerByMessageFactory = loggerByMessageFactoryByName.get(name); + if (loggerByMessageFactory == null) { + return null; + } + if (messageFactory != null) { + return loggerByMessageFactory.get(messageFactory); + } + return !loggerByMessageFactory.isEmpty() + ? loggerByMessageFactory.values().iterator().next() + : null; + } finally { + readLock.unlock(); + } } public Collection getLoggers() { @@ -131,53 +181,157 @@ public Collection getLoggers() { } public Collection getLoggers(final Collection destination) { - for (final Map inner : map.values()) { - destination.addAll(inner.values()); + requireNonNull(destination, "destination"); + readLock.lock(); + try { + loggerByMessageFactoryByName + .values() + .forEach(loggerByMessageFactory -> destination.addAll(loggerByMessageFactory.values())); + } finally { + readLock.unlock(); } return destination; } - private Map getOrCreateInnerMap(final String factoryName) { - Map inner = map.get(factoryName); - if (inner == null) { - inner = factory.createInnerMap(); - map.put(factoryName, inner); - } - return inner; - } - /** - * Detects if a Logger with the specified name exists. - * @param name The Logger name to search for. - * @return true if the Logger exists, false otherwise. + * Checks if a logger associated with the given name exists. + *

+ * There can be made no assumptions on the message factory of the found logger. + * Callers are strongly advised to switch to {@link #hasLogger(String, MessageFactory)} and provide a message factory parameter! + *

+ * + * @param name a logger name + * @return {@code true}, if the logger exists; {@code false} otherwise. + * @deprecated As of version {@code 2.24.1}, planned to be removed! + * Use {@link #hasLogger(String, MessageFactory)} instead. */ + @Deprecated public boolean hasLogger(final String name) { - return getOrCreateInnerMap(DEFAULT_FACTORY_KEY).containsKey(name); + requireNonNull(name, "name"); + final T logger = getLogger(name); + return logger != null; } /** - * Detects if a Logger with the specified name and MessageFactory exists. - * @param name The Logger name to search for. - * @param messageFactory The message factory to search for. - * @return true if the Logger exists, false otherwise. + * Checks if a logger associated with the given name and message factory exists. + *

+ * In the absence of a message factory, there can be made no assumptions on the message factory of the found logger. + * This lenient behaviour is only kept for backward compatibility. + * Callers are strongly advised to provide a message factory parameter to the method! + *

+ * + * @param name a logger name + * @param messageFactory a message factory + * @return {@code true}, if the logger exists; {@code false} otherwise. * @since 2.5 */ - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return getOrCreateInnerMap(factoryKey(messageFactory)).containsKey(name); + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + requireNonNull(name, "name"); + final T logger = getLogger(name, messageFactory); + return logger != null; } /** - * Detects if a Logger with the specified name and MessageFactory type exists. - * @param name The Logger name to search for. - * @param messageFactoryClass The message factory class to search for. - * @return true if the Logger exists, false otherwise. + * Checks if a logger associated with the given name and message factory type exists. + * + * @param name a logger name + * @param messageFactoryClass a message factory class + * @return {@code true}, if the logger exists; {@code false} otherwise. * @since 2.5 */ public boolean hasLogger(final String name, final Class messageFactoryClass) { - return getOrCreateInnerMap(factoryClassKey(messageFactoryClass)).containsKey(name); + requireNonNull(name, "name"); + requireNonNull(messageFactoryClass, "messageFactoryClass"); + readLock.lock(); + try { + return loggerByMessageFactoryByName.getOrDefault(name, Collections.emptyMap()).keySet().stream() + .anyMatch(messageFactory -> messageFactoryClass.equals(messageFactory.getClass())); + } finally { + readLock.unlock(); + } + } + + /** + * Registers the provided logger using the given name – message factory parameter is ignored and the one from the logger will be used instead. + * + * @param name a logger name + * @param messageFactory ignored – kept for backward compatibility + * @param logger a logger instance + * @deprecated As of version {@code 2.24.1}, planned to be removed! + * Use {@link #computeIfAbsent(String, MessageFactory, BiFunction)} instead. + */ + @Deprecated + public void putIfAbsent(final String name, @Nullable final MessageFactory messageFactory, final T logger) { + + // Check arguments + requireNonNull(name, "name"); + requireNonNull(logger, "logger"); + + // Insert the logger + writeLock.lock(); + try { + final Map loggerByMessageFactory = + loggerByMessageFactoryByName.computeIfAbsent(name, this::createLoggerByMessageFactoryMap); + final MessageFactory loggerMessageFactory = logger.getMessageFactory(); + loggerByMessageFactory.putIfAbsent(loggerMessageFactory, logger); + } finally { + writeLock.unlock(); + } + } + + public T computeIfAbsent( + final String name, + final MessageFactory messageFactory, + final BiFunction loggerSupplier) { + + // Check arguments + requireNonNull(name, "name"); + requireNonNull(messageFactory, "messageFactory"); + requireNonNull(loggerSupplier, "loggerSupplier"); + + // Read lock fast path: See if logger already exists + T logger = getLogger(name, messageFactory); + if (logger != null) { + return logger; + } + + // Write lock slow path: Insert the logger + writeLock.lock(); + try { + + // See if the logger is created by another thread in the meantime + final Map loggerByMessageFactory = + loggerByMessageFactoryByName.computeIfAbsent(name, this::createLoggerByMessageFactoryMap); + if ((logger = loggerByMessageFactory.get(messageFactory)) != null) { + return logger; + } + + // Create the logger + logger = loggerSupplier.apply(name, messageFactory); + + // Report message factory mismatches, if there is any + final MessageFactory loggerMessageFactory = logger.getMessageFactory(); + if (!loggerMessageFactory.equals(messageFactory)) { + StatusLogger.getLogger() + .error( + "Newly registered logger with name `{}` and message factory `{}`, is requested to be associated with a different message factory: `{}`.\n" + + "Effectively the message factory of the logger will be used and the other one will be ignored.\n" + + "This generally hints a problem at the logger context implementation.\n" + + "Please report this using the Log4j project issue tracker.", + name, + loggerMessageFactory, + messageFactory); + } + + // Insert the logger + loggerByMessageFactory.put(loggerMessageFactory, logger); + return logger; + } finally { + writeLock.unlock(); + } } - public void putIfAbsent(final String name, final MessageFactory messageFactory, final T logger) { - factory.putIfAbsent(getOrCreateInnerMap(factoryKey(messageFactory)), name, logger); + private Map createLoggerByMessageFactoryMap(final String ignored) { + return new WeakHashMap<>(); } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/package-info.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/package-info.java index 8fff7b80978..3b6b5c25857 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/package-info.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/package-info.java @@ -19,7 +19,7 @@ * API classes. */ @Export -@Version("2.24.0") +@Version("2.25.0") package org.apache.logging.log4j.spi; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 3aa72f0d169..28d8a40b7b5 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -48,13 +48,16 @@ import org.apache.logging.log4j.core.util.NetUtils; import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.spi.LoggerContextFactory; import org.apache.logging.log4j.spi.LoggerContextShutdownAware; import org.apache.logging.log4j.spi.LoggerContextShutdownEnabled; import org.apache.logging.log4j.spi.LoggerRegistry; import org.apache.logging.log4j.spi.Terminable; import org.apache.logging.log4j.spi.ThreadContextMapFactory; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.PropertiesUtil; +import org.jspecify.annotations.Nullable; /** * The LoggerContext is the anchor for the logging system. It maintains a list of all the loggers requested by @@ -90,6 +93,7 @@ public class LoggerContext extends AbstractLifeCycle private String contextName; private volatile URI configLocation; private Cancellable shutdownCallback; + private MessageFactory defaultMessageFactory = ParameterizedMessageFactory.INSTANCE; private final Lock configLock = new ReentrantLock(); @@ -514,24 +518,16 @@ public Collection getLoggers() { } /** - * Obtains a Logger from the Context. + * Obtains a logger from the context. * - * @param name The name of the Logger to return. - * @param messageFactory The message factory is used only when creating a logger, subsequent use does not change the - * logger but will log a warning if mismatched. - * @return The Logger. + * @param name a logger name + * @param messageFactory a message factory to associate the logger with + * @return a logger matching the given name and message factory */ @Override - public Logger getLogger(final String name, final MessageFactory messageFactory) { - // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. - Logger logger = loggerRegistry.getLogger(name, messageFactory); - if (logger != null) { - return logger; - } - - logger = newInstance(this, name, messageFactory); - loggerRegistry.putIfAbsent(name, messageFactory, logger); - return loggerRegistry.getLogger(name, messageFactory); + public Logger getLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = messageFactory != null ? messageFactory : defaultMessageFactory; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::newInstance); } /** @@ -552,7 +548,7 @@ public LoggerRegistry getLoggerRegistry() { */ @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name); + return loggerRegistry.hasLogger(name, defaultMessageFactory); } /** @@ -562,8 +558,9 @@ public boolean hasLogger(final String name) { * @return True if the Logger exists, false otherwise. */ @Override - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return loggerRegistry.hasLogger(name, messageFactory); + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = messageFactory != null ? messageFactory : defaultMessageFactory; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } /** @@ -648,6 +645,11 @@ public Configuration setConfiguration(final Configuration config) { // AsyncLoggers update their nanoClock when the configuration changes Log4jLogEvent.setNanoClock(configuration.getNanoClock()); + // Our implementations tend to choose a different message factory based on the employed configuration. + // Hence, creating a throwaway logger to determine the default message factory. + defaultMessageFactory = + newInstance(this, "throwaway-for-determining-MF", null).getMessageFactory(); + return prev; } finally { configLock.unlock(); @@ -810,6 +812,22 @@ private void initApiModule() { .init(); // Or make public and call ThreadContext.init() which calls ThreadContextMapFactory.init(). } + private Logger newInstance(final String name, final MessageFactory messageFactory) { + final Logger logger = newInstance(this, name, messageFactory); + final MessageFactory loggerMessageFactory = logger.getMessageFactory(); + if (!loggerMessageFactory.equals(messageFactory)) { + StatusLogger.getLogger() + .error( + "Newly created logger with name `{}` and message factory `{}` was actually requested to be created with a different message factory: `{}`.\n" + + "This generally hints a problem.\n" + + "Please report this using the Log4j project issue tracker.", + name, + loggerMessageFactory, + messageFactory); + } + return logger; + } + // LOG4J2-151: changed visibility from private to protected protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) { return new Logger(ctx, name, messageFactory); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/package-info.java index 92bb0d5449b..6eed83a54e9 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/package-info.java @@ -18,7 +18,7 @@ * Provides Asynchronous Logger classes and interfaces for low-latency logging. */ @Export -@Version("2.24.0") +@Version("2.24.1") package org.apache.logging.log4j.core.async; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/package-info.java index 266256b4637..7c02b12fe60 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/package-info.java @@ -18,7 +18,7 @@ * Implementation of Log4j 2. */ @Export -@Version("2.24.0") +@Version("2.24.1") package org.apache.logging.log4j.core; import org.osgi.annotation.bundle.Export; diff --git a/log4j-taglib/pom.xml b/log4j-taglib/pom.xml index f34353488ab..03e52ba803d 100644 --- a/log4j-taglib/pom.xml +++ b/log4j-taglib/pom.xml @@ -28,14 +28,19 @@ Apache Log4j Tag Library The Apache Log4j Tag Library for Web Applications - + + + org.jspecify.*;resolution:=optional + javax.servlet.api;substitute="javax.servlet-api";static=true;transitive=false, javax.servlet.jsp.api;substitute="javax.servlet.jsp-api";static=true;transitive=false + + org.jspecify;transitive=false org.apache.logging.log4j.core @@ -59,6 +64,11 @@ log4j-web true + + org.jspecify + jspecify + provided + org.apache.logging.log4j log4j-core diff --git a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java index 1a38afd578c..8203f9c7360 100644 --- a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java +++ b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java @@ -16,13 +16,20 @@ */ package org.apache.logging.log4j.taglib; +import java.util.Map; import java.util.WeakHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.servlet.ServletContext; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.LoggerRegistry; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** * This bridge between the tag library and the Log4j API ensures that instances of {@link Log4jTaglibLogger} are @@ -30,13 +37,21 @@ * * @since 2.0 */ +@NullMarked final class Log4jTaglibLoggerContext implements LoggerContext { - // These were change to WeakHashMaps to avoid ClassLoader (memory) leak, something that's particularly - // important in Servlet containers. - private static final WeakHashMap CONTEXTS = new WeakHashMap<>(); - private final LoggerRegistry loggerRegistry = - new LoggerRegistry<>(new LoggerRegistry.WeakMapFactory()); + private static final ReadWriteLock LOCK = new ReentrantReadWriteLock(); + + private static final Lock READ_LOCK = LOCK.readLock(); + + private static final Lock WRITE_LOCK = LOCK.writeLock(); + + private static final Map LOGGER_CONTEXT_BY_SERVLET_CONTEXT = + new WeakHashMap<>(); + + private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE; + + private final LoggerRegistry loggerRegistry = new LoggerRegistry<>(); private final ServletContext servletContext; @@ -46,35 +61,25 @@ private Log4jTaglibLoggerContext(final ServletContext servletContext) { @Override public Object getExternalContext() { - return this.servletContext; + return servletContext; } @Override public Log4jTaglibLogger getLogger(final String name) { - return this.getLogger(name, null); + return getLogger(name, null); } @Override - public Log4jTaglibLogger getLogger(final String name, final MessageFactory messageFactory) { - // Note: This is the only method where we add entries to the 'loggerRegistry' ivar. - Log4jTaglibLogger logger = this.loggerRegistry.getLogger(name, messageFactory); - if (logger != null) { - return logger; - } - - synchronized (this.loggerRegistry) { - logger = this.loggerRegistry.getLogger(name, messageFactory); - if (logger == null) { - final LoggerContext context = LogManager.getContext(false); - final ExtendedLogger original = - messageFactory == null ? context.getLogger(name) : context.getLogger(name, messageFactory); - // wrap a logger from an underlying implementation - logger = new Log4jTaglibLogger(original, name, original.getMessageFactory()); - this.loggerRegistry.putIfAbsent(name, messageFactory, logger); - } - } + public Log4jTaglibLogger getLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::createLogger); + } - return logger; + private Log4jTaglibLogger createLogger(final String name, @Nullable final MessageFactory messageFactory) { + final LoggerContext loggerContext = LogManager.getContext(false); + final ExtendedLogger delegateLogger = loggerContext.getLogger(name, messageFactory); + return new Log4jTaglibLogger(delegateLogger, name, delegateLogger.getMessageFactory()); } @Override @@ -83,8 +88,10 @@ public boolean hasLogger(final String name) { } @Override - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return loggerRegistry.hasLogger(name, messageFactory); + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } @Override @@ -92,20 +99,25 @@ public boolean hasLogger(final String name, final ClassMichael Vorburger.ch for Google */ class JULLoggerContext implements LoggerContext { + private final LoggerRegistry loggerRegistry = new LoggerRegistry<>(); + private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE; + // This implementation is strongly inspired by org.apache.logging.slf4j.SLF4JLoggerContext @Override @@ -40,29 +45,31 @@ public Object getExternalContext() { @Override public ExtendedLogger getLogger(final String name) { - if (!loggerRegistry.hasLogger(name)) { - loggerRegistry.putIfAbsent(name, null, new JULLogger(name, Logger.getLogger(name))); - } - return loggerRegistry.getLogger(name); + return getLogger(name, null); } @Override - public ExtendedLogger getLogger(final String name, final MessageFactory messageFactory) { - if (!loggerRegistry.hasLogger(name, messageFactory)) { - loggerRegistry.putIfAbsent( - name, messageFactory, new JULLogger(name, messageFactory, Logger.getLogger(name))); - } - return loggerRegistry.getLogger(name, messageFactory); + public ExtendedLogger getLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, JULLoggerContext::createLogger); + } + + private static ExtendedLogger createLogger(final String name, @Nullable final MessageFactory messageFactory) { + final Logger logger = Logger.getLogger(name); + return new JULLogger(name, messageFactory, logger); } @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name); + return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY); } @Override - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return loggerRegistry.hasLogger(name, messageFactory); + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } @Override diff --git a/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/package-info.java b/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/package-info.java index aa117c7995c..8308d5c2faf 100644 --- a/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/package-info.java +++ b/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/package-info.java @@ -21,7 +21,7 @@ * @author Michael Vorburger.ch for Google */ @Export -@Version("2.24.0") +@Version("2.24.1") package org.apache.logging.log4j.tojul; import org.osgi.annotation.bundle.Export; diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java index f0aee0af414..4e4567d1efc 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java @@ -17,14 +17,20 @@ package org.apache.logging.slf4j; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.LoggerRegistry; +import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class SLF4JLoggerContext implements LoggerContext { + private final LoggerRegistry loggerRegistry = new LoggerRegistry<>(); + private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE; + @Override public Object getExternalContext() { return null; @@ -32,29 +38,31 @@ public Object getExternalContext() { @Override public ExtendedLogger getLogger(final String name) { - if (!loggerRegistry.hasLogger(name)) { - loggerRegistry.putIfAbsent(name, null, new SLF4JLogger(name, LoggerFactory.getLogger(name))); - } - return loggerRegistry.getLogger(name); + return getLogger(name, null); } @Override - public ExtendedLogger getLogger(final String name, final MessageFactory messageFactory) { - if (!loggerRegistry.hasLogger(name, messageFactory)) { - loggerRegistry.putIfAbsent( - name, messageFactory, new SLF4JLogger(name, messageFactory, LoggerFactory.getLogger(name))); - } - return loggerRegistry.getLogger(name, messageFactory); + public ExtendedLogger getLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, SLF4JLoggerContext::createLogger); + } + + private static ExtendedLogger createLogger(final String name, @Nullable final MessageFactory messageFactory) { + final Logger logger = LoggerFactory.getLogger(name); + return new SLF4JLogger(name, messageFactory, logger); } @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name); + return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY); } @Override - public boolean hasLogger(final String name, final MessageFactory messageFactory) { - return loggerRegistry.hasLogger(name, messageFactory); + public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } @Override diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java index ba4cb130be1..12e4bedba29 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java @@ -18,7 +18,7 @@ * SLF4J support. */ @Export -@Version("2.24.0") +@Version("2.24.1") package org.apache.logging.slf4j; import org.osgi.annotation.bundle.Export; From e54dd2d976c36aac0f1d9e7b5dc557073fe813b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 13:00:53 +0200 Subject: [PATCH 05/17] Have weak references to `Logger`s in `LoggerRegistry` --- .../logging/log4j/spi/LoggerRegistry.java | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java index eac316fc448..e0955ee0892 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java @@ -18,6 +18,7 @@ import static java.util.Objects.requireNonNull; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -30,6 +31,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.BiFunction; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.status.StatusLogger; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -40,7 +42,7 @@ @NullMarked public class LoggerRegistry { - private final Map> loggerByMessageFactoryByName = new HashMap<>(); + private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); private final ReadWriteLock lock = new ReentrantReadWriteLock(); @@ -161,16 +163,15 @@ public T getLogger(final String name, @Nullable final MessageFactory messageFact requireNonNull(name, "name"); readLock.lock(); try { - final Map loggerByMessageFactory = loggerByMessageFactoryByName.get(name); - if (loggerByMessageFactory == null) { + final Map> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.get(name); + if (loggerRefByMessageFactory == null) { return null; } - if (messageFactory != null) { - return loggerByMessageFactory.get(messageFactory); - } - return !loggerByMessageFactory.isEmpty() - ? loggerByMessageFactory.values().iterator().next() - : null; + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE; + final WeakReference loggerRef = loggerRefByMessageFactory.get(effectiveMessageFactory); + return loggerRef.get(); } finally { readLock.unlock(); } @@ -184,9 +185,14 @@ public Collection getLoggers(final Collection destination) { requireNonNull(destination, "destination"); readLock.lock(); try { - loggerByMessageFactoryByName + loggerRefByMessageFactoryByName.values().forEach(loggerRefByMessageFactory -> loggerRefByMessageFactory .values() - .forEach(loggerByMessageFactory -> destination.addAll(loggerByMessageFactory.values())); + .forEach(loggerRef -> { + final T logger = loggerRef.get(); + if (logger != null) { + destination.add(logger); + } + })); } finally { readLock.unlock(); } @@ -244,7 +250,7 @@ public boolean hasLogger(final String name, final Class messageFactoryClass.equals(messageFactory.getClass())); } finally { readLock.unlock(); @@ -270,10 +276,13 @@ public void putIfAbsent(final String name, @Nullable final MessageFactory messag // Insert the logger writeLock.lock(); try { - final Map loggerByMessageFactory = - loggerByMessageFactoryByName.computeIfAbsent(name, this::createLoggerByMessageFactoryMap); + final Map> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap); final MessageFactory loggerMessageFactory = logger.getMessageFactory(); - loggerByMessageFactory.putIfAbsent(loggerMessageFactory, logger); + final WeakReference loggerRef = loggerRefByMessageFactory.get(loggerMessageFactory); + if (loggerRef == null || loggerRef.get() == null) { + loggerRefByMessageFactory.put(loggerMessageFactory, new WeakReference<>(logger)); + } } finally { writeLock.unlock(); } @@ -300,9 +309,11 @@ public T computeIfAbsent( try { // See if the logger is created by another thread in the meantime - final Map loggerByMessageFactory = - loggerByMessageFactoryByName.computeIfAbsent(name, this::createLoggerByMessageFactoryMap); - if ((logger = loggerByMessageFactory.get(messageFactory)) != null) { + final Map> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap); + final WeakReference loggerRef; + if ((loggerRef = loggerRefByMessageFactory.get(messageFactory)) != null + && (logger = loggerRef.get()) != null) { return logger; } @@ -324,14 +335,14 @@ public T computeIfAbsent( } // Insert the logger - loggerByMessageFactory.put(loggerMessageFactory, logger); + loggerRefByMessageFactory.put(loggerMessageFactory, new WeakReference<>(logger)); return logger; } finally { writeLock.unlock(); } } - private Map createLoggerByMessageFactoryMap(final String ignored) { + private Map> createLoggerRefByMessageFactoryMap(final String ignored) { return new WeakHashMap<>(); } } From bffd5902b307fbbb5ea8a833108eb1e8551fb438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 13:04:00 +0200 Subject: [PATCH 06/17] Switch version bumps from `2.24.1` to `2.25.0` --- .../apache/logging/log4j/simple/package-info.java | 2 +- .../apache/logging/log4j/spi/AbstractLogger.java | 2 +- .../apache/logging/log4j/spi/LoggerRegistry.java | 14 +++++++------- .../logging/log4j/core/async/package-info.java | 2 +- .../apache/logging/log4j/core/package-info.java | 2 +- .../apache/logging/log4j/taglib/package-info.java | 2 +- .../apache/logging/log4j/tojul/package-info.java | 2 +- .../org/apache/logging/slf4j/package-info.java | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java index 29139a12ace..6e77cad0a60 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/package-info.java @@ -20,7 +20,7 @@ * Providers are able to be loaded at runtime. */ @Export -@Version("2.24.1") +@Version("2.25.0") package org.apache.logging.log4j.simple; import org.osgi.annotation.bundle.Export; diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java index a92a4cd06d3..06d99f3f15a 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java @@ -166,7 +166,7 @@ private static MessageFactory2 adaptMessageFactory(final MessageFactory result) * * @param logger The logger to check * @param messageFactory The message factory to check. - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! * Instead, in {@link LoggerContext#getLogger(String, MessageFactory)} implementations, namespace loggers with message factories. * If your implementation uses {@link LoggerRegistry}, you are already covered. */ diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java index e0955ee0892..56f2c14e8c6 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java @@ -54,7 +54,7 @@ public class LoggerRegistry { * Data structure contract for the internal storage of admitted loggers. * * @param subtype of {@code ExtendedLogger} - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! */ @Deprecated public interface MapFactory { @@ -70,7 +70,7 @@ public interface MapFactory { * {@link MapFactory} implementation using {@link ConcurrentHashMap}. * * @param subtype of {@code ExtendedLogger} - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! */ @Deprecated public static class ConcurrentMapFactory implements MapFactory { @@ -95,7 +95,7 @@ public void putIfAbsent(final Map innerMap, final String name, final * {@link MapFactory} implementation using {@link WeakHashMap}. * * @param subtype of {@code ExtendedLogger} - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! */ @Deprecated public static class WeakMapFactory implements MapFactory { @@ -122,7 +122,7 @@ public LoggerRegistry() {} * Constructs an instance ignoring the given the map factory. * * @param mapFactory a map factory - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! */ @Deprecated public LoggerRegistry(@Nullable final MapFactory mapFactory) { @@ -138,7 +138,7 @@ public LoggerRegistry(@Nullable final MapFactory mapFactory) { * * @param name a logger name * @return the logger associated with the name - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! * Use {@link #getLogger(String, MessageFactory)} instead. */ @Deprecated @@ -208,7 +208,7 @@ public Collection getLoggers(final Collection destination) { * * @param name a logger name * @return {@code true}, if the logger exists; {@code false} otherwise. - * @deprecated As of version {@code 2.24.1}, planned to be removed! + * @deprecated As of version {@code 2.25.0}, planned to be removed! * Use {@link #hasLogger(String, MessageFactory)} instead. */ @Deprecated @@ -263,7 +263,7 @@ public boolean hasLogger(final String name, final ClassMichael Vorburger.ch for Google */ @Export -@Version("2.24.1") +@Version("2.25.0") package org.apache.logging.log4j.tojul; import org.osgi.annotation.bundle.Export; diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java index 12e4bedba29..8e6a1c3ac84 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/package-info.java @@ -18,7 +18,7 @@ * SLF4J support. */ @Export -@Version("2.24.1") +@Version("2.25.0") package org.apache.logging.slf4j; import org.osgi.annotation.bundle.Export; From c747f4fe1edc7a0e83259462ce103afc39cbbee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 13:31:30 +0200 Subject: [PATCH 07/17] Implement `equals()` and `hashCode()` for `LocalizedMessageFactory` --- .../message/LocalizedMessageFactory.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java index d8360dff0e6..a235c5723e9 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java @@ -16,6 +16,9 @@ */ package org.apache.logging.log4j.message; +import org.jspecify.annotations.Nullable; + +import java.util.Objects; import java.util.ResourceBundle; /** @@ -33,8 +36,11 @@ public class LocalizedMessageFactory extends AbstractMessageFactory { private static final long serialVersionUID = -1996295808703146741L; + @Nullable // FIXME: cannot use ResourceBundle name for serialization until Java 8 private final transient ResourceBundle resourceBundle; + + @Nullable private final String baseName; public LocalizedMessageFactory(final ResourceBundle resourceBundle) { @@ -92,4 +98,21 @@ public Message newMessage(final String key, final Object... params) { } return new LocalizedMessage(resourceBundle, key, params); } + + @Override + public boolean equals(final Object object) { + if (this == object) { + return true; + } + if (object == null || getClass() != object.getClass()) { + return false; + } + final LocalizedMessageFactory that = (LocalizedMessageFactory) object; + return Objects.equals(resourceBundle, that.resourceBundle) && Objects.equals(baseName, that.baseName); + } + + @Override + public int hashCode() { + return Objects.hash(resourceBundle, baseName); + } } From 2e443df9967852649ca5c9906e7ae26115927b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 13:39:42 +0200 Subject: [PATCH 08/17] Fix Spotless failures --- .../apache/logging/log4j/message/LocalizedMessageFactory.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java index a235c5723e9..10d75fc9072 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessageFactory.java @@ -16,10 +16,9 @@ */ package org.apache.logging.log4j.message; -import org.jspecify.annotations.Nullable; - import java.util.Objects; import java.util.ResourceBundle; +import org.jspecify.annotations.Nullable; /** * Creates {@link FormattedMessage} instances for {@link MessageFactory2} methods (and {@link MessageFactory} by From fce83c2184fb1208535f40d563272ced118497e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 11 Sep 2024 14:43:48 +0200 Subject: [PATCH 09/17] Fix `bnd-baseline` failures --- .../java/org/apache/logging/log4j/message/package-info.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/package-info.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/package-info.java index 816466b4675..e2346e8055e 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/package-info.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/package-info.java @@ -19,7 +19,7 @@ * Public Message Types used for Log4j 2. Users may implement their own Messages. */ @Export -@Version("2.24.0") +@Version("2.25.0") package org.apache.logging.log4j.message; import org.osgi.annotation.bundle.Export; From 712fe91f2d40afce102480aba91ceeec1a5f2511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 12 Sep 2024 13:04:36 +0200 Subject: [PATCH 10/17] Improve `LoggerContext#defaultMessageFactory` docs --- .../logging/log4j/core/LoggerContext.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 28d8a40b7b5..2c58d1f45b2 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -93,6 +93,14 @@ public class LoggerContext extends AbstractLifeCycle private String contextName; private volatile URI configLocation; private Cancellable shutdownCallback; + + /** + * The default message factory to use while creating loggers. + *

+ * The initial value is only set to avoid nullability. + * The actual value is populated by {@link #reloadDefaultMessageFactory()} during (re)configuration. + *

+ */ private MessageFactory defaultMessageFactory = ParameterizedMessageFactory.INSTANCE; private final Lock configLock = new ReentrantLock(); @@ -645,10 +653,7 @@ public Configuration setConfiguration(final Configuration config) { // AsyncLoggers update their nanoClock when the configuration changes Log4jLogEvent.setNanoClock(configuration.getNanoClock()); - // Our implementations tend to choose a different message factory based on the employed configuration. - // Hence, creating a throwaway logger to determine the default message factory. - defaultMessageFactory = - newInstance(this, "throwaway-for-determining-MF", null).getMessageFactory(); + reloadDefaultMessageFactory(); return prev; } finally { @@ -656,6 +661,18 @@ public Configuration setConfiguration(final Configuration config) { } } + /** + * Reloads the value of {@link #defaultMessageFactory}. + *

+ * {@link LoggerContext} implementations tend to choose a different message factory based on the employed configuration. + * Hence, this method creates a throwaway logger to determine the default message factory. + *

+ */ + private void reloadDefaultMessageFactory() { + defaultMessageFactory = + newInstance(this, "throwaway-for-determining-MF", null).getMessageFactory(); + } + private static void registerJmxBeans() { if (!isJmxDisabled()) { try { From cc0e729c9233f4fd92494e65417af3ba30e4bc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 13 Sep 2024 14:05:23 +0200 Subject: [PATCH 11/17] Apply review feedback --- .../logging/log4j/spi/LoggerRegistry.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java index 56f2c14e8c6..323055fb40a 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.Lock; @@ -171,6 +172,9 @@ public T getLogger(final String name, @Nullable final MessageFactory messageFact final MessageFactory effectiveMessageFactory = messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE; final WeakReference loggerRef = loggerRefByMessageFactory.get(effectiveMessageFactory); + if (loggerRef == null) { + return null; + } return loggerRef.get(); } finally { readLock.unlock(); @@ -185,14 +189,11 @@ public Collection getLoggers(final Collection destination) { requireNonNull(destination, "destination"); readLock.lock(); try { - loggerRefByMessageFactoryByName.values().forEach(loggerRefByMessageFactory -> loggerRefByMessageFactory - .values() - .forEach(loggerRef -> { - final T logger = loggerRef.get(); - if (logger != null) { - destination.add(logger); - } - })); + loggerRefByMessageFactoryByName.values().stream() + .flatMap(loggerRefByMessageFactory -> + loggerRefByMessageFactory.values().stream().map(WeakReference::get)) + .filter(Objects::nonNull) + .forEach(destination::add); } finally { readLock.unlock(); } From 3668f5de8a6f3a6a7f1d6e88481d588e2cf904f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 13 Sep 2024 14:09:37 +0200 Subject: [PATCH 12/17] Improve changelog --- ...2936_deprecate_AbstractLogger_checkMessageFactory.xml | 8 ++++++++ ...936_make_LoggerRegistry_MessageFactory_namespaced.xml | 9 +++++++++ 2 files changed, 17 insertions(+) create mode 100644 src/changelog/.2.x.x/2936_deprecate_AbstractLogger_checkMessageFactory.xml create mode 100644 src/changelog/.2.x.x/2936_make_LoggerRegistry_MessageFactory_namespaced.xml diff --git a/src/changelog/.2.x.x/2936_deprecate_AbstractLogger_checkMessageFactory.xml b/src/changelog/.2.x.x/2936_deprecate_AbstractLogger_checkMessageFactory.xml new file mode 100644 index 00000000000..e72a33566a4 --- /dev/null +++ b/src/changelog/.2.x.x/2936_deprecate_AbstractLogger_checkMessageFactory.xml @@ -0,0 +1,8 @@ + + + + Deprecate `AbstractLogger.checkMessageFactory()`, since all created `Logger`s are already `MessageFactory`-namespaced + diff --git a/src/changelog/.2.x.x/2936_make_LoggerRegistry_MessageFactory_namespaced.xml b/src/changelog/.2.x.x/2936_make_LoggerRegistry_MessageFactory_namespaced.xml new file mode 100644 index 00000000000..c314f84251d --- /dev/null +++ b/src/changelog/.2.x.x/2936_make_LoggerRegistry_MessageFactory_namespaced.xml @@ -0,0 +1,9 @@ + + + + Rework `LoggerRegistry` to make it `MessageFactory`-namespaced. +This effectively allows loggers of same name, but different message factory. + From a8007767db6f80278a2045b39bba7faa4094ce4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Sun, 15 Sep 2024 21:21:16 +0200 Subject: [PATCH 13/17] Lazy initialize the default `MF` in `LC` --- .../logging/log4j/core/LoggerContext.java | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 2c58d1f45b2..0ac4c741bd7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -48,7 +48,6 @@ import org.apache.logging.log4j.core.util.NetUtils; import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry; import org.apache.logging.log4j.message.MessageFactory; -import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.spi.LoggerContextFactory; import org.apache.logging.log4j.spi.LoggerContextShutdownAware; import org.apache.logging.log4j.spi.LoggerContextShutdownEnabled; @@ -56,6 +55,7 @@ import org.apache.logging.log4j.spi.Terminable; import org.apache.logging.log4j.spi.ThreadContextMapFactory; import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.util.Lazy; import org.apache.logging.log4j.util.PropertiesUtil; import org.jspecify.annotations.Nullable; @@ -96,12 +96,11 @@ public class LoggerContext extends AbstractLifeCycle /** * The default message factory to use while creating loggers. - *

- * The initial value is only set to avoid nullability. - * The actual value is populated by {@link #reloadDefaultMessageFactory()} during (re)configuration. - *

*/ - private MessageFactory defaultMessageFactory = ParameterizedMessageFactory.INSTANCE; + private final Lazy defaultMessageFactoryRef = Lazy.lazy(() -> { + final Logger throwawayLogger = newInstance(this, "throwaway-for-determining-MF", null); + return throwawayLogger.getMessageFactory(); + }); private final Lock configLock = new ReentrantLock(); @@ -534,7 +533,8 @@ public Collection getLoggers() { */ @Override public Logger getLogger(final String name, @Nullable final MessageFactory messageFactory) { - final MessageFactory effectiveMessageFactory = messageFactory != null ? messageFactory : defaultMessageFactory; + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : defaultMessageFactoryRef.get(); return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::newInstance); } @@ -556,7 +556,7 @@ public LoggerRegistry getLoggerRegistry() { */ @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name, defaultMessageFactory); + return loggerRegistry.hasLogger(name, defaultMessageFactoryRef.get()); } /** @@ -567,7 +567,8 @@ public boolean hasLogger(final String name) { */ @Override public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { - final MessageFactory effectiveMessageFactory = messageFactory != null ? messageFactory : defaultMessageFactory; + final MessageFactory effectiveMessageFactory = + messageFactory != null ? messageFactory : defaultMessageFactoryRef.get(); return loggerRegistry.hasLogger(name, effectiveMessageFactory); } @@ -653,26 +654,12 @@ public Configuration setConfiguration(final Configuration config) { // AsyncLoggers update their nanoClock when the configuration changes Log4jLogEvent.setNanoClock(configuration.getNanoClock()); - reloadDefaultMessageFactory(); - return prev; } finally { configLock.unlock(); } } - /** - * Reloads the value of {@link #defaultMessageFactory}. - *

- * {@link LoggerContext} implementations tend to choose a different message factory based on the employed configuration. - * Hence, this method creates a throwaway logger to determine the default message factory. - *

- */ - private void reloadDefaultMessageFactory() { - defaultMessageFactory = - newInstance(this, "throwaway-for-determining-MF", null).getMessageFactory(); - } - private static void registerJmxBeans() { if (!isJmxDisabled()) { try { From c1dff86c087c9e4ae5ee7e68ad16385d02dc894e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 16 Sep 2024 11:11:24 +0200 Subject: [PATCH 14/17] Use `Logger#getEffectiveMessageFactory()` --- .../org/apache/logging/log4j/core/Logger.java | 2 +- .../logging/log4j/core/LoggerContext.java | 22 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java index 777d627115d..169c39162db 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java @@ -111,7 +111,7 @@ protected Logger( this.privateConfig = new PrivateConfig(context.getConfiguration(), this); } - private static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) { + static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) { return createInstanceFromFactoryProperty( MessageFactory.class, messageFactory, diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 0ac4c741bd7..1e6c3a47fe5 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -55,7 +55,6 @@ import org.apache.logging.log4j.spi.Terminable; import org.apache.logging.log4j.spi.ThreadContextMapFactory; import org.apache.logging.log4j.status.StatusLogger; -import org.apache.logging.log4j.util.Lazy; import org.apache.logging.log4j.util.PropertiesUtil; import org.jspecify.annotations.Nullable; @@ -78,6 +77,13 @@ public class LoggerContext extends AbstractLifeCycle private static final Configuration NULL_CONFIGURATION = new NullConfiguration(); + /** + * The default message factory to use while creating loggers, if none is provided. + * + * @see #2936 for the discussion on why we leak the message factory of the default logger and hardcode it here. + */ + private static final MessageFactory DEFAULT_MESSAGE_FACTORY = Logger.getEffectiveMessageFactory(null); + private final LoggerRegistry loggerRegistry = new LoggerRegistry<>(); private final CopyOnWriteArrayList propertyChangeListeners = new CopyOnWriteArrayList<>(); private volatile List listeners; @@ -94,14 +100,6 @@ public class LoggerContext extends AbstractLifeCycle private volatile URI configLocation; private Cancellable shutdownCallback; - /** - * The default message factory to use while creating loggers. - */ - private final Lazy defaultMessageFactoryRef = Lazy.lazy(() -> { - final Logger throwawayLogger = newInstance(this, "throwaway-for-determining-MF", null); - return throwawayLogger.getMessageFactory(); - }); - private final Lock configLock = new ReentrantLock(); /** @@ -534,7 +532,7 @@ public Collection getLoggers() { @Override public Logger getLogger(final String name, @Nullable final MessageFactory messageFactory) { final MessageFactory effectiveMessageFactory = - messageFactory != null ? messageFactory : defaultMessageFactoryRef.get(); + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::newInstance); } @@ -556,7 +554,7 @@ public LoggerRegistry getLoggerRegistry() { */ @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name, defaultMessageFactoryRef.get()); + return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY); } /** @@ -568,7 +566,7 @@ public boolean hasLogger(final String name) { @Override public boolean hasLogger(final String name, @Nullable final MessageFactory messageFactory) { final MessageFactory effectiveMessageFactory = - messageFactory != null ? messageFactory : defaultMessageFactoryRef.get(); + messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY; return loggerRegistry.hasLogger(name, effectiveMessageFactory); } From c581239a447ff613de524e105091b9f3b7cd298b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 16 Sep 2024 12:25:30 +0200 Subject: [PATCH 15/17] Fix Mockito failures --- pom.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pom.xml b/pom.xml index 3ac428a3a22..e778a2e640d 100644 --- a/pom.xml +++ b/pom.xml @@ -963,14 +963,30 @@ + + java8-tests + env.CI true + + + + + org.jspecify + jspecify + test + + + @@ -990,6 +1006,7 @@ + From 3a3e02c465346b42b93c531abee5b5a11ef63c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 16 Sep 2024 12:42:50 +0200 Subject: [PATCH 16/17] Fix BND typo in `log4j-taglib/pom.xml` Co-authored-by: Piotr P. Karwasz --- log4j-taglib/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log4j-taglib/pom.xml b/log4j-taglib/pom.xml index 03e52ba803d..3855d8f441f 100644 --- a/log4j-taglib/pom.xml +++ b/log4j-taglib/pom.xml @@ -38,7 +38,7 @@ javax.servlet.api;substitute="javax.servlet-api";static=true;transitive=false, - javax.servlet.jsp.api;substitute="javax.servlet.jsp-api";static=true;transitive=false + javax.servlet.jsp.api;substitute="javax.servlet.jsp-api";static=true;transitive=false, org.jspecify;transitive=false From 952a9a53e961eac7afde7e726a1e3f8605cf7880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 16 Sep 2024 13:14:47 +0200 Subject: [PATCH 17/17] Remove redundant changelog entry --- .../deprecate_AbstractLogger_checkMessageFactory.xml | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml diff --git a/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml b/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml deleted file mode 100644 index e72a33566a4..00000000000 --- a/src/changelog/.2.x.x/deprecate_AbstractLogger_checkMessageFactory.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - Deprecate `AbstractLogger.checkMessageFactory()`, since all created `Logger`s are already `MessageFactory`-namespaced -