You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2024/09/16 18:56:04 UTC
(logging-log4j2) branch fix/2.24.x/LoggerRegistry-revamp created (now 03e4f652c0)
This is an automated email from the ASF dual-hosted git repository.
vy pushed a change to branch fix/2.24.x/LoggerRegistry-revamp
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
at 03e4f652c0 Implement `MessageFactory`-namespaced logger registry
This branch includes the following new commits:
new 03e4f652c0 Implement `MessageFactory`-namespaced logger registry
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(logging-log4j2) 01/01: Implement `MessageFactory`-namespaced logger registry
Posted by vy...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch fix/2.24.x/LoggerRegistry-revamp
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 03e4f652c02b109ed15105d8d4fc2bfae3a2667a
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Mon Sep 16 20:56:26 2024 +0200
Implement `MessageFactory`-namespaced logger registry
---
.../log4j/message/LocalizedMessageFactory.java | 22 +++
.../apache/logging/log4j/message/package-info.java | 2 +-
.../logging/log4j/simple/SimpleLoggerContext.java | 37 ++--
.../apache/logging/log4j/simple/package-info.java | 2 +-
.../apache/logging/log4j/spi/LoggerRegistry.java | 212 +++++++++++++++------
.../org/apache/logging/log4j/spi/package-info.java | 2 +-
.../java/org/apache/logging/log4j/core/Logger.java | 2 +-
.../apache/logging/log4j/core/LoggerContext.java | 57 ++++--
.../apache/logging/log4j/core/package-info.java | 2 +-
.../log4j/taglib/Log4jTaglibLoggerContext.java | 89 +++++----
.../apache/logging/log4j/taglib/package-info.java | 2 +-
.../logging/log4j/tojul/JULLoggerContext.java | 32 +++-
.../apache/logging/log4j/tojul/package-info.java | 2 +-
.../apache/logging/slf4j/SLF4JLoggerContext.java | 33 +++-
.../org/apache/logging/slf4j/package-info.java | 2 +-
...ke_LoggerRegistry_MessageFactory_namespaced.xml | 9 +
16 files changed, 349 insertions(+), 158 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 d8360dff0e..10d75fc907 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 @@ import java.util.ResourceBundle;
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 class LocalizedMessageFactory extends AbstractMessageFactory {
}
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 816466b467..dae91264f4 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.24.1")
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 d1aaf8db0f..7196b93ceb 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 edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
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 <code>SimpleLog</code> 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,20 @@ public class SimpleLoggerContext implements LoggerContext {
}
@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;
+ public ExtendedLogger getLogger(final String name, @Nullable final MessageFactory messageFactory) {
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ final ExtendedLogger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
+ if (oldLogger != null) {
+ return oldLogger;
}
- final SimpleLogger simpleLogger = new SimpleLogger(
+ final ExtendedLogger newLogger = createLogger(name, effectiveMessageFactory);
+ loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
+ return loggerRegistry.getLogger(name, effectiveMessageFactory);
+ }
+
+ private ExtendedLogger createLogger(final String name, @Nullable final MessageFactory messageFactory) {
+ return new SimpleLogger(
name,
defaultLevel,
showLogName,
@@ -114,8 +123,6 @@ public class SimpleLoggerContext implements LoggerContext {
messageFactory,
props,
stream);
- loggerRegistry.putIfAbsent(name, messageFactory, simpleLogger);
- return loggerRegistry.getLogger(name, messageFactory);
}
/**
@@ -131,16 +138,18 @@ public class SimpleLoggerContext implements LoggerContext {
@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<? extends MessageFactory> 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 1481df9462..29139a12ac 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/LoggerRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java
index e71c0a933d..0e04484ff1 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,43 @@
*/
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 org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
/**
- * Convenience class to be used by {@code LoggerContext} implementations.
+ * Convenience class to be used as an {@link ExtendedLogger} registry by {@code LoggerContext} implementations.
*/
public class LoggerRegistry<T extends ExtendedLogger> {
- private static final String DEFAULT_FACTORY_KEY = AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS.getName();
- private final MapFactory<T> factory;
- private final Map<String, Map<String, T>> map;
+
+ private final Map<String, Map<MessageFactory, WeakReference<T>>> 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 <T> subtype of {@code ExtendedLogger}
*/
public interface MapFactory<T extends ExtendedLogger> {
+
Map<String, T> createInnerMap();
Map<String, Map<String, T>> createOuterMap();
@@ -46,10 +61,12 @@ public class LoggerRegistry<T extends ExtendedLogger> {
}
/**
- * Generates ConcurrentHashMaps for use by the registry to store the Loggers.
+ * {@link MapFactory} implementation using {@link ConcurrentHashMap}.
+ *
* @param <T> subtype of {@code ExtendedLogger}
*/
public static class ConcurrentMapFactory<T extends ExtendedLogger> implements MapFactory<T> {
+
@Override
public Map<String, T> createInnerMap() {
return new ConcurrentHashMap<>();
@@ -62,15 +79,17 @@ public class LoggerRegistry<T extends ExtendedLogger> {
@Override
public void putIfAbsent(final Map<String, T> innerMap, final String name, final T logger) {
- ((ConcurrentMap<String, T>) 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 <T> subtype of {@code ExtendedLogger}
*/
public static class WeakMapFactory<T extends ExtendedLogger> implements MapFactory<T> {
+
@Override
public Map<String, T> createInnerMap() {
return new WeakHashMap<>();
@@ -87,43 +106,63 @@ public class LoggerRegistry<T extends ExtendedLogger> {
}
}
- public LoggerRegistry() {
- this(new ConcurrentMapFactory<T>());
- }
-
- public LoggerRegistry(final MapFactory<T> factory) {
- this.factory = Objects.requireNonNull(factory, "factory");
- this.map = factory.createOuterMap();
- }
+ public LoggerRegistry() {}
- private static String factoryClassKey(final Class<? extends MessageFactory> 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 <b>ignoring</b> the given the map factory.
+ *
+ * @param mapFactory a map factory
+ */
+ public LoggerRegistry(final MapFactory<T> 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.
+ * <p>
+ * 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 <b>provide a message factory parameter!</b>
+ * </p>
+ *
+ * @param name a logger name
+ * @return the logger associated with the name
*/
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.
+ * <p>
+ * 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 <b>provide a message factory parameter to the method!</b>
+ * </p>
+ *
+ * @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);
+ requireNonNull(name, "name");
+ readLock.lock();
+ try {
+ final Map<MessageFactory, WeakReference<T>> loggerRefByMessageFactory =
+ loggerRefByMessageFactoryByName.get(name);
+ if (loggerRefByMessageFactory == null) {
+ return null;
+ }
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE;
+ final WeakReference<T> loggerRef = loggerRefByMessageFactory.get(effectiveMessageFactory);
+ if (loggerRef == null) {
+ return null;
+ }
+ return loggerRef.get();
+ } finally {
+ readLock.unlock();
+ }
}
public Collection<T> getLoggers() {
@@ -131,53 +170,104 @@ public class LoggerRegistry<T extends ExtendedLogger> {
}
public Collection<T> getLoggers(final Collection<T> destination) {
- for (final Map<String, T> 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<String, T> getOrCreateInnerMap(final String factoryName) {
- Map<String, T> 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.
+ * <p>
+ * 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 <b>provide a message factory parameter!</b>
+ * </p>
+ *
+ * @param name a logger name
+ * @return {@code true}, if the logger exists; {@code false} otherwise.
*/
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.
+ * <p>
+ * 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 <b>provide a message factory parameter to the method!</b>
+ * </p>
+ *
+ * @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);
+ 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<? extends MessageFactory> 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 – <b>message factory parameter is ignored</b> 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
+ */
public void putIfAbsent(final String name, final MessageFactory messageFactory, final T logger) {
- factory.putIfAbsent(getOrCreateInnerMap(factoryKey(messageFactory)), name, logger);
+
+ // Check arguments
+ requireNonNull(name, "name");
+ requireNonNull(logger, "logger");
+
+ // Insert the logger
+ writeLock.lock();
+ try {
+ final Map<MessageFactory, WeakReference<T>> loggerRefByMessageFactory =
+ loggerRefByMessageFactoryByName.computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap);
+ final MessageFactory loggerMessageFactory = logger.getMessageFactory();
+ final WeakReference<T> loggerRef = loggerRefByMessageFactory.get(loggerMessageFactory);
+ if (loggerRef == null || loggerRef.get() == null) {
+ loggerRefByMessageFactory.put(loggerMessageFactory, new WeakReference<>(logger));
+ }
+ } finally {
+ writeLock.unlock();
+ }
+ }
+
+ private Map<MessageFactory, WeakReference<T>> 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 8fff7b8097..1748932083 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.24.1")
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 777d627115..169c39162d 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 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
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 07cd2d1f59..2289266eae 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,13 @@ import org.apache.logging.log4j.core.util.ExecutorServices;
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;
/**
@@ -76,6 +76,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 <a href="https://github.com/apache/logging-log4j2/pull/2936">#2936</a> 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<Logger> loggerRegistry = new LoggerRegistry<>();
private final CopyOnWriteArrayList<PropertyChangeListener> propertyChangeListeners = new CopyOnWriteArrayList<>();
private volatile List<LoggerContextShutdownAware> listeners;
@@ -515,25 +522,23 @@ public class LoggerContext extends AbstractLifeCycle
}
/**
- * 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;
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ final Logger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
+ if (oldLogger != null) {
+ return oldLogger;
}
-
- logger = newInstance(this, name, messageFactory);
- loggerRegistry.putIfAbsent(name, messageFactory, logger);
- return loggerRegistry.getLogger(name, messageFactory);
+ final Logger newLogger = newInstance(name, effectiveMessageFactory);
+ loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
+ return loggerRegistry.getLogger(name, effectiveMessageFactory);
}
/**
@@ -554,7 +559,7 @@ public class LoggerContext extends AbstractLifeCycle
*/
@Override
public boolean hasLogger(final String name) {
- return loggerRegistry.hasLogger(name);
+ return loggerRegistry.hasLogger(name, DEFAULT_MESSAGE_FACTORY);
}
/**
@@ -565,7 +570,9 @@ public class LoggerContext extends AbstractLifeCycle
*/
@Override
public boolean hasLogger(final String name, final MessageFactory messageFactory) {
- return loggerRegistry.hasLogger(name, messageFactory);
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ return loggerRegistry.hasLogger(name, effectiveMessageFactory);
}
/**
@@ -812,6 +819,22 @@ public class LoggerContext extends AbstractLifeCycle
.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/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/package-info.java
index 266256b463..7c02b12fe6 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/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java
index 3c76f39b21..30f1a1f029 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,11 +16,15 @@
*/
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;
@@ -32,12 +36,19 @@ import org.apache.logging.log4j.spi.LoggerRegistry;
* @since 2.0
*/
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<ServletContext, Log4jTaglibLoggerContext> CONTEXTS = new WeakHashMap<>();
- private final LoggerRegistry<Log4jTaglibLogger> loggerRegistry =
- new LoggerRegistry<>(new LoggerRegistry.WeakMapFactory<Log4jTaglibLogger>());
+ 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<ServletContext, Log4jTaglibLoggerContext> LOGGER_CONTEXT_BY_SERVLET_CONTEXT =
+ new WeakHashMap<>();
+
+ private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE;
+
+ private final LoggerRegistry<Log4jTaglibLogger> loggerRegistry = new LoggerRegistry<>();
private final ServletContext servletContext;
@@ -47,36 +58,31 @@ final class Log4jTaglibLoggerContext implements LoggerContext {
@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);
- }
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ final Log4jTaglibLogger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
+ if (oldLogger != null) {
+ return oldLogger;
}
+ final Log4jTaglibLogger newLogger = createLogger(name, effectiveMessageFactory);
+ loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
+ return loggerRegistry.getLogger(name, effectiveMessageFactory);
+ }
- return logger;
+ private Log4jTaglibLogger createLogger(final String name, final MessageFactory messageFactory) {
+ final LoggerContext loggerContext = LogManager.getContext(false);
+ final ExtendedLogger delegateLogger = loggerContext.getLogger(name, messageFactory);
+ return new Log4jTaglibLogger(delegateLogger, name, delegateLogger.getMessageFactory());
}
@Override
@@ -86,7 +92,9 @@ final class Log4jTaglibLoggerContext implements LoggerContext {
@Override
public boolean hasLogger(final String name, final MessageFactory messageFactory) {
- return loggerRegistry.hasLogger(name, messageFactory);
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ return loggerRegistry.hasLogger(name, effectiveMessageFactory);
}
@Override
@@ -94,20 +102,25 @@ final class Log4jTaglibLoggerContext implements LoggerContext {
return loggerRegistry.hasLogger(name, messageFactoryClass);
}
- static synchronized Log4jTaglibLoggerContext getInstance(final ServletContext servletContext) {
- Log4jTaglibLoggerContext loggerContext = CONTEXTS.get(servletContext);
- if (loggerContext != null) {
- return loggerContext;
- }
+ static Log4jTaglibLoggerContext getInstance(final ServletContext servletContext) {
- synchronized (CONTEXTS) {
- loggerContext = CONTEXTS.get(servletContext);
- if (loggerContext == null) {
- loggerContext = new Log4jTaglibLoggerContext(servletContext);
- CONTEXTS.put(servletContext, loggerContext);
+ // Get the associated logger context, if exists
+ READ_LOCK.lock();
+ try {
+ final Log4jTaglibLoggerContext loggerContext = LOGGER_CONTEXT_BY_SERVLET_CONTEXT.get(servletContext);
+ if (loggerContext != null) {
+ return loggerContext;
}
+ } finally {
+ READ_LOCK.unlock();
}
- return loggerContext;
+ // Create the logger context
+ WRITE_LOCK.lock();
+ try {
+ return LOGGER_CONTEXT_BY_SERVLET_CONTEXT.computeIfAbsent(servletContext, Log4jTaglibLoggerContext::new);
+ } finally {
+ WRITE_LOCK.unlock();
+ }
}
}
diff --git a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/package-info.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/package-info.java
index 3614d93117..ac85ecafea 100644
--- a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/package-info.java
+++ b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/package-info.java
@@ -20,7 +20,7 @@
* @since 2.0
*/
@Export
-@Version("2.20.1")
+@Version("2.24.1")
package org.apache.logging.log4j.taglib;
import org.osgi.annotation.bundle.Export;
diff --git a/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/JULLoggerContext.java b/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/JULLoggerContext.java
index 7d2c2351c6..b041d4d195 100644
--- a/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/JULLoggerContext.java
+++ b/log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/JULLoggerContext.java
@@ -18,6 +18,7 @@ package org.apache.logging.log4j.tojul;
import java.util.logging.Logger;
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;
@@ -29,8 +30,11 @@ import org.apache.logging.log4j.spi.LoggerRegistry;
* @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
*/
class JULLoggerContext implements LoggerContext {
+
private final LoggerRegistry<ExtendedLogger> 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 +44,37 @@ class JULLoggerContext implements LoggerContext {
@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)));
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ final ExtendedLogger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
+ if (oldLogger != null) {
+ return oldLogger;
}
- return loggerRegistry.getLogger(name, messageFactory);
+ final ExtendedLogger newLogger = createLogger(name, effectiveMessageFactory);
+ loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
+ return loggerRegistry.getLogger(name, effectiveMessageFactory);
+ }
+
+ private static ExtendedLogger createLogger(final String name, 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);
+ 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 aa117c7995..8308d5c2fa 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 <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> 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 f0aee0af41..e16a9a177c 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,19 @@
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.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class SLF4JLoggerContext implements LoggerContext {
+
private final LoggerRegistry<ExtendedLogger> loggerRegistry = new LoggerRegistry<>();
+ private static final MessageFactory DEFAULT_MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE;
+
@Override
public Object getExternalContext() {
return null;
@@ -32,29 +37,37 @@ public class SLF4JLoggerContext implements LoggerContext {
@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)));
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
+ final ExtendedLogger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
+ if (oldLogger != null) {
+ return oldLogger;
}
- return loggerRegistry.getLogger(name, messageFactory);
+ final ExtendedLogger newLogger = createLogger(name, effectiveMessageFactory);
+ loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
+ return loggerRegistry.getLogger(name, effectiveMessageFactory);
+ }
+
+ private static ExtendedLogger createLogger(final String name, 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);
+ 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 ba4cb130be..12e4bedba2 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;
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 0000000000..c314f84251
--- /dev/null
+++ b/src/changelog/.2.x.x/2936_make_LoggerRegistry_MessageFactory_namespaced.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ 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="changed">
+ <issue id="2936" link="https://github.com/apache/logging-log4j2/pull/2936"/>
+ <description format="asciidoc">Rework `LoggerRegistry` to make it `MessageFactory`-namespaced.
+This effectively allows loggers of same name, but different message factory.</description>
+</entry>