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..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,7 +16,9 @@ */ package org.apache.logging.log4j.message; +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 @@ -33,8 +35,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 +97,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); + } } 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; 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..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,12 +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.AbstractLogger; 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. @@ -41,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? */ @@ -96,14 +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) { - AbstractLogger.checkMessageFactory(extendedLogger, messageFactory); - 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, @@ -114,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); } /** @@ -131,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..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.0") +@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 ecc499ac586..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,11 @@ 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.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. */ + @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-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..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 @@ -16,28 +16,50 @@ */ package org.apache.logging.log4j.spi; +import static java.util.Objects.requireNonNull; + +import java.lang.ref.WeakReference; 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.message.ParameterizedMessageFactory; +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>> loggerRefByMessageFactoryByName = 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.25.0}, planned to be removed! */ + @Deprecated public interface MapFactory { + Map createInnerMap(); Map> createOuterMap(); @@ -46,10 +68,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.25.0}, planned to be removed! */ + @Deprecated public static class ConcurrentMapFactory implements MapFactory { + @Override public Map createInnerMap() { return new ConcurrentHashMap<>(); @@ -62,15 +88,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.25.0}, planned to be removed! */ + @Deprecated public static class WeakMapFactory implements MapFactory { + @Override public Map createInnerMap() { return new WeakHashMap<>(); @@ -87,43 +117,68 @@ 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.25.0}, 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.25.0}, 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> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.get(name); + if (loggerRefByMessageFactory == null) { + return null; + } + 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(); + } } public Collection getLoggers() { @@ -131,53 +186,164 @@ 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 { + loggerRefByMessageFactoryByName.values().stream() + .flatMap(loggerRefByMessageFactory -> + loggerRefByMessageFactory.values().stream().map(WeakReference::get)) + .filter(Objects::nonNull) + .forEach(destination::add); + } 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.25.0}, 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 loggerRefByMessageFactoryByName.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.25.0}, 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> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap); + final MessageFactory loggerMessageFactory = logger.getMessageFactory(); + final WeakReference loggerRef = loggerRefByMessageFactory.get(loggerMessageFactory); + if (loggerRef == null || loggerRef.get() == null) { + loggerRefByMessageFactory.put(loggerMessageFactory, new WeakReference<>(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> loggerRefByMessageFactory = + loggerRefByMessageFactoryByName.computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap); + final WeakReference loggerRef; + if ((loggerRef = loggerRefByMessageFactory.get(messageFactory)) != null + && (logger = loggerRef.get()) != 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 + loggerRefByMessageFactory.put(loggerMessageFactory, new WeakReference<>(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> createLoggerRefByMessageFactoryMap(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/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 07cd2d1f592..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 @@ -48,14 +48,15 @@ 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; 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 @@ -76,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; @@ -515,25 +523,17 @@ 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) { - AbstractLogger.checkMessageFactory(logger, messageFactory); - 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 : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::newInstance); } /** @@ -554,7 +554,7 @@ public LoggerRegistry getLoggerRegistry() { */ @Override public boolean hasLogger(final String name) { - return loggerRegistry.hasLogger(name); + return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY); } /** @@ -564,8 +564,10 @@ 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 : DEFAULT_MESSAGE_FACTORY; + return loggerRegistry.hasLogger(name, effectiveMessageFactory); } /** @@ -812,6 +814,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..abf19fcd6f4 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.25.0") 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..171f9c7e78d 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.25.0") 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..3855d8f441f 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 + 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 3c76f39b21a..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,14 +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.spi.AbstractLogger; +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 @@ -31,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; @@ -47,36 +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) { - AbstractLogger.checkMessageFactory(logger, messageFactory); - 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 @@ -85,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 @@ -94,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..68812bb248b 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.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/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..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.0") +@Version("2.25.0") package org.apache.logging.slf4j; import org.osgi.annotation.bundle.Export; 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 @@ + 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. +