You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/11/21 22:42:54 UTC
(logging-log4j2) branch 2.x-site-pro updated (539e008ffc -> cdb83102d3)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a change to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
from 539e008ffc Fix code typo in the thread context docs (#3186)
new 26538096e0 Bump version to `2.24.2-SNAPSHOT`
new f8a2ef7d0e Removes weak references from `LoggerRepository` (`2.24.x` branch) (#3209)
new 95abfb9c14 Fix key removal issues in Thread Context (#3210)
new 34ccf4c0a8 Fix off-by-one in `StackTraceStringResolver` (#3216)
new f92b3a542d Prepare release notes for version `2.24.2`
new cdb83102d3 Update the `project.build.outputTimestamp` property
The 6 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.
Summary of changes:
log4j-api-test/pom.xml | 5 +
.../logging/log4j/CloseableThreadContextTest.java | 68 ++++++++
.../map/UnmodifiableArrayBackedMapTest.java | 20 +++
.../logging/log4j/spi/LoggerRegistryTest.java | 64 +++++++
.../internal/map/UnmodifiableArrayBackedMap.java | 100 +++++------
.../apache/logging/log4j/spi/LoggerRegistry.java | 68 ++++----
.../org/apache/logging/log4j/spi/package-info.java | 2 +-
.../apache/logging/log4j/core/LoggerContext.java | 26 ++-
.../core/util/internal/InternalLoggerRegistry.java | 186 +++++++++++++++++++++
.../resolver/StackTraceStringResolverTest.java | 63 +++++++
.../json/resolver/StackTraceStringResolver.java | 25 ++-
pom.xml | 10 +-
src/changelog/2.24.1/.release-notes.adoc.ftl | 11 +-
src/changelog/2.24.2/.release-notes.adoc.ftl | 29 ++++
src/changelog/{2.23.0 => 2.24.2}/.release.xml | 2 +-
.../3048_fix_ThreadContext_remove.xml} | 3 +-
.../3143_logger_registry.xml} | 4 +-
.../3216_fix_JTL_StackTraceStringResolver.xml} | 4 +-
18 files changed, 555 insertions(+), 135 deletions(-)
create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/spi/LoggerRegistryTest.java
create mode 100644 log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistry.java
create mode 100644 src/changelog/2.24.2/.release-notes.adoc.ftl
copy src/changelog/{2.23.0 => 2.24.2}/.release.xml (95%)
copy src/changelog/{2.0-beta2/DefaultConfiguration_was_not_starting_the_Console_Appender.xml => 2.24.2/3048_fix_ThreadContext_remove.xml} (64%)
copy src/changelog/{2.21.0/1312_add_environment_arbiter.xml => 2.24.2/3143_logger_registry.xml} (71%)
copy src/changelog/{2.0.1/LOG4J2-731_Updated_documentation_regarding_extensions_to_LoggerContextF.xml => 2.24.2/3216_fix_JTL_StackTraceStringResolver.xml} (59%)
(logging-log4j2) 02/06: Removes weak references from `LoggerRepository` (`2.24.x` branch) (#3209)
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit f8a2ef7d0ef149540831a825252ce86f450c86ed
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Nov 18 20:38:33 2024 +0100
Removes weak references from `LoggerRepository` (`2.24.x` branch) (#3209)
This is a port of #3199 to the 2.24.x branch.
Removes weak references to `Logger`s in `LoggerRepository`.
The usage of weak references in `LoggerRepository` might cause `null` to be returned by `LogManager.getLogger()` of all Log4j Core versions up to `2.24.1`.
Versions of Log4j API up to `2.24.0` did hold **hard** references to all the registered loggers, so the change will not alter the previous behavior.
This PR also inverts the order of the `String` and `MessageFactory` keys to the `LoggerRepository` multi-map to limit the number of internal maps. The external map is a `WeakHashMap` to allow logger-specific message factories to be GC-ed.
Closes #3143.
Co-authored-by: Volkan Yazıcı <vo...@yazi.ci>
---
log4j-api-test/pom.xml | 5 +
.../logging/log4j/spi/LoggerRegistryTest.java | 64 +++++++
.../apache/logging/log4j/spi/LoggerRegistry.java | 68 ++++----
.../org/apache/logging/log4j/spi/package-info.java | 2 +-
.../apache/logging/log4j/core/LoggerContext.java | 26 ++-
.../core/util/internal/InternalLoggerRegistry.java | 186 +++++++++++++++++++++
src/changelog/.2.x.x/3143_logger_registry.xml | 10 ++
7 files changed, 310 insertions(+), 51 deletions(-)
diff --git a/log4j-api-test/pom.xml b/log4j-api-test/pom.xml
index e1fb806b9b..aef8a1b5d6 100644
--- a/log4j-api-test/pom.xml
+++ b/log4j-api-test/pom.xml
@@ -144,6 +144,11 @@
<artifactId>mockito-inline</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.jspecify</groupId>
+ <artifactId>jspecify</artifactId>
+ <scope>test</scope>
+ </dependency>
<!-- Used by ServiceLoaderUtilTest -->
<dependency>
<groupId>org.osgi</groupId>
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/LoggerRegistryTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/LoggerRegistryTest.java
new file mode 100644
index 0000000000..269785ea71
--- /dev/null
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/LoggerRegistryTest.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.spi;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.lang.ref.WeakReference;
+import java.util.stream.Stream;
+import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.apache.logging.log4j.message.ReusableMessageFactory;
+import org.apache.logging.log4j.test.TestLogger;
+import org.jspecify.annotations.Nullable;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class LoggerRegistryTest {
+
+ private static final String LOGGER_NAME = LoggerRegistryTest.class.getName();
+
+ static Stream<@Nullable MessageFactory> doesNotLoseLoggerReferences() {
+ return Stream.of(
+ ParameterizedMessageFactory.INSTANCE,
+ ReusableMessageFactory.INSTANCE,
+ new ParameterizedMessageFactory(),
+ null);
+ }
+
+ /**
+ * @see <a href="https://github.com/apache/logging-log4j2/issues/3143>Issue #3143</a>
+ */
+ @ParameterizedTest
+ @MethodSource
+ void doesNotLoseLoggerReferences(@Nullable MessageFactory messageFactory) {
+ LoggerRegistry<TestLogger> loggerRegistry = new LoggerRegistry<>();
+ TestLogger logger = new TestLogger(LOGGER_NAME, messageFactory);
+ WeakReference<TestLogger> loggerRef = new WeakReference<>(logger);
+ // Register logger
+ loggerRegistry.putIfAbsent(LOGGER_NAME, messageFactory, logger);
+ // The JIT compiler/optimizer might figure out by himself the `logger` and `messageFactory` are no longer used:
+ // https://shipilev.net/jvm/anatomy-quarks/8-local-var-reachability/
+ // We help him with the task though.
+ logger = null;
+ // Trigger a GC run
+ System.gc();
+ // Check if the logger is still there
+ assertThat(loggerRef.get()).isNotNull();
+ assertThat(loggerRegistry.getLogger(LOGGER_NAME, messageFactory)).isInstanceOf(TestLogger.class);
+ }
+}
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 d1ff7b9738..f7646b0273 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,27 +18,27 @@ 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.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.jspecify.annotations.Nullable;
/**
* Convenience class to be used as an {@link ExtendedLogger} registry by {@code LoggerContext} implementations.
*/
public class LoggerRegistry<T extends ExtendedLogger> {
- private final Map<String, Map<MessageFactory, WeakReference<T>>> loggerRefByMessageFactoryByName = new HashMap<>();
+ private final Map<String, Map<MessageFactory, T>> loggerByMessageFactoryByName = new HashMap<>();
private final ReadWriteLock lock = new ReentrantReadWriteLock();
@@ -127,7 +127,7 @@ public class LoggerRegistry<T extends ExtendedLogger> {
* @param name a logger name
* @return the logger associated with the name
*/
- public T getLogger(final String name) {
+ public @Nullable T getLogger(final String name) {
requireNonNull(name, "name");
return getLogger(name, null);
}
@@ -144,39 +144,29 @@ public class LoggerRegistry<T extends ExtendedLogger> {
* @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) {
+ public @Nullable T getLogger(final String name, @Nullable final MessageFactory messageFactory) {
requireNonNull(name, "name");
readLock.lock();
try {
- final Map<MessageFactory, WeakReference<T>> loggerRefByMessageFactory =
- loggerRefByMessageFactoryByName.get(name);
- if (loggerRefByMessageFactory == null) {
- return null;
- }
+ final @Nullable Map<MessageFactory, T> loggerByMessageFactory = loggerByMessageFactoryByName.get(name);
final MessageFactory effectiveMessageFactory =
messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE;
- final WeakReference<T> loggerRef = loggerRefByMessageFactory.get(effectiveMessageFactory);
- if (loggerRef == null) {
- return null;
- }
- return loggerRef.get();
+ return loggerByMessageFactory == null ? null : loggerByMessageFactory.get(effectiveMessageFactory);
} finally {
readLock.unlock();
}
}
public Collection<T> getLoggers() {
- return getLoggers(new ArrayList<T>());
+ return getLoggers(new ArrayList<>());
}
public Collection<T> getLoggers(final Collection<T> destination) {
requireNonNull(destination, "destination");
readLock.lock();
try {
- loggerRefByMessageFactoryByName.values().stream()
- .flatMap(loggerRefByMessageFactory ->
- loggerRefByMessageFactory.values().stream().map(WeakReference::get))
- .filter(Objects::nonNull)
+ loggerByMessageFactoryByName.values().stream()
+ .flatMap(loggerByMessageFactory -> loggerByMessageFactory.values().stream())
.forEach(destination::add);
} finally {
readLock.unlock();
@@ -196,7 +186,7 @@ public class LoggerRegistry<T extends ExtendedLogger> {
*/
public boolean hasLogger(final String name) {
requireNonNull(name, "name");
- final T logger = getLogger(name);
+ final @Nullable T logger = getLogger(name);
return logger != null;
}
@@ -215,7 +205,7 @@ public class LoggerRegistry<T extends ExtendedLogger> {
*/
public boolean hasLogger(final String name, final MessageFactory messageFactory) {
requireNonNull(name, "name");
- final T logger = getLogger(name, messageFactory);
+ final @Nullable T logger = getLogger(name, messageFactory);
return logger != null;
}
@@ -232,7 +222,7 @@ public class LoggerRegistry<T extends ExtendedLogger> {
requireNonNull(messageFactoryClass, "messageFactoryClass");
readLock.lock();
try {
- return loggerRefByMessageFactoryByName.getOrDefault(name, Collections.emptyMap()).keySet().stream()
+ return loggerByMessageFactoryByName.getOrDefault(name, Collections.emptyMap()).keySet().stream()
.anyMatch(messageFactory -> messageFactoryClass.equals(messageFactory.getClass()));
} finally {
readLock.unlock();
@@ -241,34 +231,42 @@ public class LoggerRegistry<T extends ExtendedLogger> {
/**
* Registers the provided logger.
- * <b>Logger name and message factory parameters are ignored</b>, those will be obtained from the logger instead.
+ * <p>
+ * The logger will be registered using the keys provided by the {@code name} and {@code messageFactory} parameters
+ * and the values of {@link Logger#getName()} and {@link Logger#getMessageFactory()}.
+ * </p>
*
- * @param name ignored – kept for backward compatibility
- * @param messageFactory ignored – kept for backward compatibility
+ * @param name a logger name
+ * @param messageFactory a message factory
* @param logger a logger instance
*/
- public void putIfAbsent(final String name, final MessageFactory messageFactory, final T logger) {
+ 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<MessageFactory, WeakReference<T>> loggerRefByMessageFactory =
- loggerRefByMessageFactoryByName.computeIfAbsent(
- logger.getName(), 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));
+ final MessageFactory effectiveMessageFactory =
+ messageFactory != null ? messageFactory : ParameterizedMessageFactory.INSTANCE;
+ // Register using the keys provided by the caller
+ loggerByMessageFactoryByName
+ .computeIfAbsent(name, this::createLoggerRefByMessageFactoryMap)
+ .putIfAbsent(effectiveMessageFactory, logger);
+ // Also register using the values extracted from `logger`
+ if (!name.equals(logger.getName()) || !effectiveMessageFactory.equals(logger.getMessageFactory())) {
+ loggerByMessageFactoryByName
+ .computeIfAbsent(logger.getName(), this::createLoggerRefByMessageFactoryMap)
+ .putIfAbsent(logger.getMessageFactory(), logger);
}
} finally {
writeLock.unlock();
}
}
- private Map<MessageFactory, WeakReference<T>> createLoggerRefByMessageFactoryMap(final String ignored) {
+ private Map<MessageFactory, 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 1748932083..1637d7a3d1 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.1")
+@Version("2.24.2")
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 88f66ac364..d084fd2484 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
@@ -33,6 +33,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.ConfigurationFactory;
@@ -47,11 +48,11 @@ import org.apache.logging.log4j.core.util.Cancellable;
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.core.util.internal.InternalLoggerRegistry;
import org.apache.logging.log4j.message.MessageFactory;
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.util.PropertiesUtil;
@@ -82,7 +83,7 @@ public class LoggerContext extends AbstractLifeCycle
*/
private static final MessageFactory DEFAULT_MESSAGE_FACTORY = Logger.getEffectiveMessageFactory(null);
- private final LoggerRegistry<Logger> loggerRegistry = new LoggerRegistry<>();
+ private final InternalLoggerRegistry loggerRegistry = new InternalLoggerRegistry();
private final CopyOnWriteArrayList<PropertyChangeListener> propertyChangeListeners = new CopyOnWriteArrayList<>();
private volatile List<LoggerContextShutdownAware> listeners;
@@ -512,7 +513,7 @@ public class LoggerContext extends AbstractLifeCycle
* @return a collection of the current loggers.
*/
public Collection<Logger> getLoggers() {
- return loggerRegistry.getLoggers();
+ return loggerRegistry.getLoggers().collect(Collectors.toList());
}
/**
@@ -526,13 +527,7 @@ public class LoggerContext extends AbstractLifeCycle
public Logger getLogger(final String name, final MessageFactory messageFactory) {
final MessageFactory effectiveMessageFactory =
messageFactory != null ? messageFactory : DEFAULT_MESSAGE_FACTORY;
- final Logger oldLogger = loggerRegistry.getLogger(name, effectiveMessageFactory);
- if (oldLogger != null) {
- return oldLogger;
- }
- final Logger newLogger = newInstance(name, effectiveMessageFactory);
- loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger);
- return loggerRegistry.getLogger(name, effectiveMessageFactory);
+ return loggerRegistry.computeIfAbsent(name, effectiveMessageFactory, this::newInstance);
}
/**
@@ -541,8 +536,11 @@ public class LoggerContext extends AbstractLifeCycle
* @return the LoggerRegistry.
* @since 2.17.2
*/
- public LoggerRegistry<Logger> getLoggerRegistry() {
- return loggerRegistry;
+ public org.apache.logging.log4j.spi.LoggerRegistry<Logger> getLoggerRegistry() {
+ org.apache.logging.log4j.spi.LoggerRegistry<Logger> result =
+ new org.apache.logging.log4j.spi.LoggerRegistry<>();
+ loggerRegistry.getLoggers().forEach(l -> result.putIfAbsent(l.getName(), l.getMessageFactory(), l));
+ return result;
}
/**
@@ -775,9 +773,7 @@ public class LoggerContext extends AbstractLifeCycle
*/
public void updateLoggers(final Configuration config) {
final Configuration old = this.configuration;
- for (final Logger logger : loggerRegistry.getLoggers()) {
- logger.updateConfiguration(config);
- }
+ loggerRegistry.getLoggers().forEach(logger -> logger.updateConfiguration(config));
firePropertyChangeEvent(new PropertyChangeEvent(this, PROPERTY_CONFIG, old, config));
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistry.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistry.java
new file mode 100644
index 0000000000..5aec88f599
--- /dev/null
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistry.java
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.util.internal;
+
+import static java.util.Objects.requireNonNull;
+
+import java.lang.ref.WeakReference;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.WeakHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.BiFunction;
+import java.util.stream.Stream;
+import org.apache.logging.log4j.core.Logger;
+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 used by {@link org.apache.logging.log4j.core.LoggerContext}
+ * <p>
+ * We don't use {@link org.apache.logging.log4j.spi.LoggerRegistry} from the Log4j API to keep Log4j Core independent
+ * from the version of the Log4j API at runtime.
+ * </p>
+ * @since 2.25.0
+ */
+@NullMarked
+public final class InternalLoggerRegistry {
+
+ private final Map<MessageFactory, Map<String, WeakReference<Logger>>> loggerRefByNameByMessageFactory =
+ new WeakHashMap<>();
+
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+ private final Lock readLock = lock.readLock();
+
+ private final Lock writeLock = lock.writeLock();
+
+ public InternalLoggerRegistry() {}
+
+ /**
+ * Returns the logger associated with the given name and message factory.
+ *
+ * @param name a logger name
+ * @param messageFactory a message factory
+ * @return the logger associated with the given name and message factory
+ */
+ public @Nullable Logger getLogger(final String name, final MessageFactory messageFactory) {
+ requireNonNull(name, "name");
+ requireNonNull(messageFactory, "messageFactory");
+ readLock.lock();
+ try {
+ return Optional.of(loggerRefByNameByMessageFactory)
+ .map(loggerRefByNameByMessageFactory -> loggerRefByNameByMessageFactory.get(messageFactory))
+ .map(loggerRefByName -> loggerRefByName.get(name))
+ .map(WeakReference::get)
+ .orElse(null);
+ } finally {
+ readLock.unlock();
+ }
+ }
+
+ public Stream<Logger> getLoggers() {
+ readLock.lock();
+ try {
+ return loggerRefByNameByMessageFactory.values().stream()
+ .flatMap(loggerRefByName -> loggerRefByName.values().stream())
+ .flatMap(loggerRef -> {
+ @Nullable Logger logger = loggerRef.get();
+ return logger != null ? Stream.of(logger) : Stream.empty();
+ });
+ } finally {
+ readLock.unlock();
+ }
+ }
+
+ /**
+ * Checks if a logger associated with the given name and message factory exists.
+ *
+ * @param name a logger name
+ * @param messageFactory a message factory
+ * @return {@code true}, if the logger exists; {@code false} otherwise.
+ */
+ public boolean hasLogger(final String name, final MessageFactory messageFactory) {
+ requireNonNull(name, "name");
+ requireNonNull(messageFactory, "messageFactory");
+ return getLogger(name, messageFactory) != null;
+ }
+
+ /**
+ * 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.
+ */
+ public boolean hasLogger(final String name, final Class<? extends MessageFactory> messageFactoryClass) {
+ requireNonNull(name, "name");
+ requireNonNull(messageFactoryClass, "messageFactoryClass");
+ readLock.lock();
+ try {
+ return loggerRefByNameByMessageFactory.entrySet().stream()
+ .filter(entry -> messageFactoryClass.equals(entry.getKey().getClass()))
+ .anyMatch(entry -> entry.getValue().containsKey(name));
+ } finally {
+ readLock.unlock();
+ }
+ }
+
+ public Logger computeIfAbsent(
+ final String name,
+ final MessageFactory messageFactory,
+ final BiFunction<String, MessageFactory, Logger> loggerSupplier) {
+
+ // Check arguments
+ requireNonNull(name, "name");
+ requireNonNull(messageFactory, "messageFactory");
+ requireNonNull(loggerSupplier, "loggerSupplier");
+
+ // Read lock fast path: See if logger already exists
+ @Nullable Logger 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<String, WeakReference<Logger>> loggerRefByName =
+ loggerRefByNameByMessageFactory.computeIfAbsent(messageFactory, ignored -> new HashMap<>());
+ WeakReference<Logger> loggerRef = loggerRefByName.get(name);
+ if (loggerRef != null && (logger = loggerRef.get()) != null) {
+ return logger;
+ }
+
+ // Create the logger
+ logger = loggerSupplier.apply(name, messageFactory);
+
+ // Report name and message factory mismatch if there are any
+ final String loggerName = logger.getName();
+ 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 name `{}` or 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.",
+ loggerName,
+ loggerMessageFactory,
+ name,
+ messageFactory);
+ // Register logger under alternative keys
+ loggerRefByNameByMessageFactory
+ .computeIfAbsent(loggerMessageFactory, ignored -> new HashMap<>())
+ .putIfAbsent(loggerName, new WeakReference<>(logger));
+ }
+
+ // Insert the logger
+ loggerRefByName.put(name, new WeakReference<>(logger));
+ return logger;
+ } finally {
+ writeLock.unlock();
+ }
+ }
+}
diff --git a/src/changelog/.2.x.x/3143_logger_registry.xml b/src/changelog/.2.x.x/3143_logger_registry.xml
new file mode 100644
index 0000000000..39691cf8c8
--- /dev/null
+++ b/src/changelog/.2.x.x/3143_logger_registry.xml
@@ -0,0 +1,10 @@
+<?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="fixed">
+ <issue id="3143" link="https://github.com/apache/logging-log4j2/issues/3143"/>
+ <description format="asciidoc">
+ Use hard references to `Logger`s in `LoggerRegistry`.
+ </description>
+</entry>
(logging-log4j2) 05/06: Prepare release notes for version `2.24.2`
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit f92b3a542dd992515ac273ff547c8d99f7837048
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Nov 18 21:27:38 2024 +0100
Prepare release notes for version `2.24.2`
---
pom.xml | 8 +++---
src/changelog/2.24.1/.release-notes.adoc.ftl | 11 +++++++-
src/changelog/2.24.2/.release-notes.adoc.ftl | 29 ++++++++++++++++++++++
src/changelog/2.24.2/.release.xml | 21 ++++++++++++++++
.../3048_fix_ThreadContext_remove.xml | 0
.../{.2.x.x => 2.24.2}/3143_logger_registry.xml | 2 +-
.../3216_fix_JTL_StackTraceStringResolver.xml | 0
7 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/pom.xml b/pom.xml
index 16e79e4bb0..e01e9f3cc3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -306,11 +306,11 @@
<properties>
<!-- project version -->
- <revision>2.24.2-SNAPSHOT</revision>
+ <revision>2.24.2</revision>
<!-- Versions used on the site: no snapshots! -->
- <site-log4j-api.version>2.24.1</site-log4j-api.version>
- <site-log4j-core.version>2.24.1</site-log4j-core.version>
- <site-log4j-layout-template-json.version>2.24.1</site-log4j-layout-template-json.version>
+ <site-log4j-api.version>2.24.2</site-log4j-api.version>
+ <site-log4j-core.version>2.24.2</site-log4j-core.version>
+ <site-log4j-layout-template-json.version>2.24.2</site-log4j-layout-template-json.version>
<!-- =================
Common properties
diff --git a/src/changelog/2.24.1/.release-notes.adoc.ftl b/src/changelog/2.24.1/.release-notes.adoc.ftl
index 81f6635324..4fc8a69381 100644
--- a/src/changelog/2.24.1/.release-notes.adoc.ftl
+++ b/src/changelog/2.24.1/.release-notes.adoc.ftl
@@ -14,12 +14,21 @@
See the License for the specific language governing permissions and
limitations under the License.
////
-
+<#-- @ftlvariable name="release.date" type="java.lang.String" -->
+<#-- @ftlvariable name="release.version" type="java.lang.String" -->
[#release-notes-${release.version?replace("[^a-zA-Z0-9]", "-", "r")}]
== ${release.version}
<#if release.date?has_content>Release date:: ${release.date}</#if>
+[WARNING]
+====
+Due to a critical bug affecting this release (see
+https://github.com/apache/logging-log4j2/issues/3143[#3143])
+users are encouraged to upgrade to
+${'<<release-notes-2-24-2, version `2.24.2`>>'}.
+====
+
This release contains mainly bug fixes of problems encountered with the thread context map, logger registry and configuration reloading.
It also enhances integration tests to use Docker images of the most recent releases of MongoDB and Elastic Search.
diff --git a/src/changelog/2.24.2/.release-notes.adoc.ftl b/src/changelog/2.24.2/.release-notes.adoc.ftl
new file mode 100644
index 0000000000..70aba0a62d
--- /dev/null
+++ b/src/changelog/2.24.2/.release-notes.adoc.ftl
@@ -0,0 +1,29 @@
+////
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+////
+<#-- @ftlvariable name="release.date" type="java.lang.String" -->
+<#-- @ftlvariable name="release.version" type="java.lang.String" -->
+[#release-notes-${release.version?replace("[^a-zA-Z0-9]", "-", "r")}]
+== ${release.version}
+
+<#if release.date?has_content>Release date:: ${release.date}</#if>
+
+This release fixes a critical bug in Log4j API initialization code, which can cause `LogManager.getLogger()` to return `null` under certain conditions.
+See
+https://github.com/apache/logging-log4j2/issues/3143[#3143]
+for details.
+
+<#include "../.changelog.adoc.ftl">
diff --git a/src/changelog/2.24.2/.release.xml b/src/changelog/2.24.2/.release.xml
new file mode 100644
index 0000000000..d011d5bf83
--- /dev/null
+++ b/src/changelog/2.24.2/.release.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to you under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+<release xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ date="2024-11-18" version="2.24.2"/>
diff --git a/src/changelog/.2.x.x/3048_fix_ThreadContext_remove.xml b/src/changelog/2.24.2/3048_fix_ThreadContext_remove.xml
similarity index 100%
rename from src/changelog/.2.x.x/3048_fix_ThreadContext_remove.xml
rename to src/changelog/2.24.2/3048_fix_ThreadContext_remove.xml
diff --git a/src/changelog/.2.x.x/3143_logger_registry.xml b/src/changelog/2.24.2/3143_logger_registry.xml
similarity index 87%
rename from src/changelog/.2.x.x/3143_logger_registry.xml
rename to src/changelog/2.24.2/3143_logger_registry.xml
index 39691cf8c8..f30a9be729 100644
--- a/src/changelog/.2.x.x/3143_logger_registry.xml
+++ b/src/changelog/2.24.2/3143_logger_registry.xml
@@ -5,6 +5,6 @@
type="fixed">
<issue id="3143" link="https://github.com/apache/logging-log4j2/issues/3143"/>
<description format="asciidoc">
- Use hard references to `Logger`s in `LoggerRegistry`.
+ Use hard references to ``Logger``s in `LoggerRegistry`.
</description>
</entry>
diff --git a/src/changelog/.2.x.x/3216_fix_JTL_StackTraceStringResolver.xml b/src/changelog/2.24.2/3216_fix_JTL_StackTraceStringResolver.xml
similarity index 100%
rename from src/changelog/.2.x.x/3216_fix_JTL_StackTraceStringResolver.xml
rename to src/changelog/2.24.2/3216_fix_JTL_StackTraceStringResolver.xml
(logging-log4j2) 04/06: Fix off-by-one in `StackTraceStringResolver` (#3216)
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 34ccf4c0a82a0cad4cf4fda936608eb93c084b3f
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Mon Nov 18 20:51:33 2024 +0100
Fix off-by-one in `StackTraceStringResolver` (#3216)
Fixes #3194, ports #3212.
---
.../resolver/StackTraceStringResolverTest.java | 63 ++++++++++++++++++++++
.../json/resolver/StackTraceStringResolver.java | 25 +++++----
.../3216_fix_JTL_StackTraceStringResolver.xml | 8 +++
3 files changed, 83 insertions(+), 13 deletions(-)
diff --git a/log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolverTest.java b/log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolverTest.java
index 430d6de646..dd3b371e10 100644
--- a/log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolverTest.java
+++ b/log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolverTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.layout.template.json.resolver;
+import static java.util.Collections.singletonList;
import static org.apache.logging.log4j.layout.template.json.TestHelpers.CONFIGURATION;
import static org.apache.logging.log4j.layout.template.json.TestHelpers.JAVA_BASE_PREFIX;
import static org.apache.logging.log4j.layout.template.json.TestHelpers.asMap;
@@ -40,6 +41,7 @@ import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.layout.template.json.JsonTemplateLayout;
import org.apache.logging.log4j.layout.template.json.JsonTemplateLayoutDefaults;
+import org.apache.logging.log4j.layout.template.json.util.TruncatingBufferedPrintWriter;
import org.apache.logging.log4j.util.Constants;
import org.assertj.core.api.AbstractStringAssert;
import org.junit.jupiter.api.Nested;
@@ -593,6 +595,67 @@ class StackTraceStringResolverTest {
private String matchingRegex(final String string) {
return "[" + string.charAt(0) + "]" + Pattern.quote(string.substring(1));
}
+
+ @Test
+ void should_not_fail_on_truncated_output_not_ending_with_newline() {
+
+ // Try to find an exception whose truncated stack trace does not end with a newline
+ final int maxStringLength = 100;
+ final float maxByteCountPerChar =
+ JsonTemplateLayoutDefaults.getCharset().newEncoder().maxBytesPerChar();
+ final int maxStringByteCount =
+ Math.toIntExact(Math.round(Math.ceil(maxByteCountPerChar * maxStringLength)));
+ final TruncatingBufferedPrintWriter writer = TruncatingBufferedPrintWriter.ofCapacity(maxStringByteCount);
+ Exception exception;
+ String message = "m";
+ do {
+ exception = new Exception(message);
+ exception.printStackTrace(writer);
+ if (writer.truncated() && writer.buffer()[writer.length() - 1] != '\n') {
+ break;
+ }
+ writer.close();
+ message += "m";
+ } while (true);
+
+ // Create the event template
+ final String eventTemplate = writeJson(asMap(
+ "ex",
+ asMap(
+ "$resolver",
+ "exception",
+ "field",
+ "stackTrace",
+ "stackTrace",
+ asMap(
+ "stringified",
+ asMap(
+ "truncation",
+ asMap(
+ "suffix",
+ TRUNCATION_SUFFIX,
+ "pointMatcherStrings",
+ singletonList("this string shouldn't match with anything")))))));
+
+ // Create the layout
+ final JsonTemplateLayout layout = JsonTemplateLayout.newBuilder()
+ .setConfiguration(CONFIGURATION)
+ .setEventTemplate(eventTemplate)
+ .setMaxStringLength(maxStringLength)
+ .setStackTraceEnabled(true)
+ .build();
+
+ // Create the log event
+ final LogEvent logEvent =
+ Log4jLogEvent.newBuilder().setThrown(exception).build();
+
+ // Check the serialized event
+ usingSerializedLogEventAccessor(layout, logEvent, accessor -> {
+ final int expectedStackTraceLength = maxStringLength + TRUNCATION_SUFFIX.length();
+ final String stackTrace = accessor.getString("ex");
+ assertThat(stackTrace).hasSizeLessThan(expectedStackTraceLength);
+ });
+ }
}
@Test
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolver.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolver.java
index ebd34a3a47..7a4634a86f 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolver.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/StackTraceStringResolver.java
@@ -119,7 +119,7 @@ final class StackTraceStringResolver implements StackTraceResolver {
for (; ; ) {
// Find the next label start, if present.
- final int labeledLineStartIndex = findLabeledLineStartIndex(srcWriter, startIndex, srcWriter.length());
+ final int labeledLineStartIndex = findLabeledLineStartIndex(srcWriter, startIndex);
final int endIndex = labeledLineStartIndex >= 0 ? labeledLineStartIndex : srcWriter.length();
// Copy up to the truncation point, if it matches.
@@ -195,26 +195,27 @@ final class StackTraceStringResolver implements StackTraceResolver {
return -1;
}
- private static int findLabeledLineStartIndex(final CharSequence buffer, final int startIndex, final int endIndex) {
+ private static int findLabeledLineStartIndex(final CharSequence buffer, final int startIndex) {
// Note that the index arithmetic in this method is not guarded.
// That is, there are no `Math.addExact()` or `Math.subtractExact()` usages.
// Since we know a priori that we are already operating within buffer limits.
- for (int bufferIndex = startIndex; bufferIndex < endIndex; ) {
+ final int bufferLength = buffer.length();
+ for (int bufferIndex = startIndex; bufferIndex < bufferLength; ) {
// Find the next line start, if exists.
- final int lineStartIndex = findLineStartIndex(buffer, bufferIndex, endIndex);
+ final int lineStartIndex = findLineStartIndex(buffer, bufferIndex);
if (lineStartIndex < 0) {
break;
}
bufferIndex = lineStartIndex;
// Skip tabs.
- while (bufferIndex < endIndex && '\t' == buffer.charAt(bufferIndex)) {
+ while (bufferIndex < bufferLength && '\t' == buffer.charAt(bufferIndex)) {
bufferIndex++;
}
// Search for the `Caused by: ` occurrence.
- if (bufferIndex < (endIndex - 11)
+ if (bufferIndex < (bufferLength - 11)
&& buffer.charAt(bufferIndex) == 'C'
&& buffer.charAt(bufferIndex + 1) == 'a'
&& buffer.charAt(bufferIndex + 2) == 'u'
@@ -230,7 +231,7 @@ final class StackTraceStringResolver implements StackTraceResolver {
}
// Search for the `Suppressed: ` occurrence.
- else if (bufferIndex < (endIndex - 12)
+ else if (bufferIndex < (bufferLength - 12)
&& buffer.charAt(bufferIndex) == 'S'
&& buffer.charAt(bufferIndex + 1) == 'u'
&& buffer.charAt(bufferIndex + 2) == 'p'
@@ -249,13 +250,11 @@ final class StackTraceStringResolver implements StackTraceResolver {
return -1;
}
- private static int findLineStartIndex(final CharSequence buffer, final int startIndex, final int endIndex) {
- char prevChar = '-';
- for (int i = startIndex; i <= endIndex; i++) {
- if (prevChar == '\n') {
- return i;
+ private static int findLineStartIndex(final CharSequence buffer, final int startIndex) {
+ for (int bufferIndex = startIndex; bufferIndex < buffer.length(); bufferIndex++) {
+ if (buffer.charAt(bufferIndex) == '\n' && (bufferIndex + 1) < buffer.length()) {
+ return bufferIndex + 1;
}
- prevChar = buffer.charAt(i);
}
return -1;
}
diff --git a/src/changelog/.2.x.x/3216_fix_JTL_StackTraceStringResolver.xml b/src/changelog/.2.x.x/3216_fix_JTL_StackTraceStringResolver.xml
new file mode 100644
index 0000000000..55e62f9fad
--- /dev/null
+++ b/src/changelog/.2.x.x/3216_fix_JTL_StackTraceStringResolver.xml
@@ -0,0 +1,8 @@
+<?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="fixed">
+ <issue id="3216" link="https://github.com/apache/logging-log4j2/pull/3216"/>
+ <description format="asciidoc">Fix `ArrayIndexOutOfBoundsException` in JSON Template Layout truncated exception resolver</description>
+</entry>
(logging-log4j2) 06/06: Update the `project.build.outputTimestamp` property
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit cdb83102d360bfbd428b8bbfba412acffcc25586
Author: ASF Logging Services RM <pr...@logging.apache.org>
AuthorDate: Mon Nov 18 21:02:09 2024 +0000
Update the `project.build.outputTimestamp` property
---
pom.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pom.xml b/pom.xml
index e01e9f3cc3..5cd3ca5de9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -324,7 +324,7 @@
2. This value is employed in various places while creating the distribution
To mitigate these, we define a *dummy* value here and let the CI replace it during a release.
Hence, *DO NOT MANUALLY EDIT THIS VALUE*! -->
- <project.build.outputTimestamp>2024-09-24T13:33:09Z</project.build.outputTimestamp>
+ <project.build.outputTimestamp>2024-11-18T20:27:38Z</project.build.outputTimestamp>
<!-- ========================
Site-specific properties
(logging-log4j2) 03/06: Fix key removal issues in Thread Context (#3210)
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 95abfb9c14af34bcc61556a1c34accf5069aeb49
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Nov 18 20:40:13 2024 +0100
Fix key removal issues in Thread Context (#3210)
This backports #3050 to the 2.24.x branch.
Co-authored-by: Volkan Yazıcı <vo...@yazi.ci>
---
.../logging/log4j/CloseableThreadContextTest.java | 68 ++++++++++++++
.../map/UnmodifiableArrayBackedMapTest.java | 20 +++++
.../internal/map/UnmodifiableArrayBackedMap.java | 100 +++++++++------------
.../.2.x.x/3048_fix_ThreadContext_remove.xml | 8 ++
4 files changed, 137 insertions(+), 59 deletions(-)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
index 56e0086a49..bc053b080c 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
@@ -19,7 +19,9 @@ package org.apache.logging.log4j;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -244,4 +246,70 @@ public class CloseableThreadContextTest {
}
assertEquals("", ThreadContext.peek());
}
+
+ /**
+ * User provided test stressing nesting using {@link CloseableThreadContext#put(String, String)}.
+ *
+ * @see <a href="https://github.com/apache/logging-log4j2/issues/2946#issuecomment-2382935426">#2946</a>
+ */
+ @Test
+ void testAutoCloseableThreadContextPut() {
+ try (final CloseableThreadContext.Instance ctc1 = CloseableThreadContext.put("outer", "one")) {
+ try (final CloseableThreadContext.Instance ctc2 = CloseableThreadContext.put("outer", "two")) {
+ assertEquals("two", ThreadContext.get("outer"));
+
+ try (final CloseableThreadContext.Instance ctc3 = CloseableThreadContext.put("inner", "one")) {
+ assertEquals("one", ThreadContext.get("inner"));
+
+ ThreadContext.put(
+ "not-in-closeable", "true"); // Remove this line, and closing context behaves as expected
+ assertEquals("two", ThreadContext.get("outer"));
+ }
+
+ assertEquals("two", ThreadContext.get("outer"));
+ assertNull(ThreadContext.get("inner")); // Test fails here
+ }
+
+ assertEquals("one", ThreadContext.get("outer"));
+ assertNull(ThreadContext.get("inner"));
+ }
+ assertEquals("true", ThreadContext.get("not-in-closeable"));
+
+ assertNull(ThreadContext.get("inner"));
+ assertNull(ThreadContext.get("outer"));
+ }
+
+ /**
+ * User provided test stressing nesting using {@link CloseableThreadContext#putAll(Map)}.
+ *
+ * @see <a href="https://github.com/apache/logging-log4j2/issues/2946#issuecomment-2382935426">#2946</a>
+ */
+ @Test
+ void testAutoCloseableThreadContextPutAll() {
+ try (final CloseableThreadContext.Instance ctc1 = CloseableThreadContext.put("outer", "one")) {
+ try (final CloseableThreadContext.Instance ctc2 = CloseableThreadContext.put("outer", "two")) {
+ assertEquals("two", ThreadContext.get("outer"));
+
+ try (final CloseableThreadContext.Instance ctc3 = CloseableThreadContext.put("inner", "one")) {
+ assertEquals("one", ThreadContext.get("inner"));
+
+ ThreadContext.put(
+ "not-in-closeable", "true"); // Remove this line, and closing context behaves as expected
+ ThreadContext.putAll(Collections.singletonMap("inner", "two")); // But this is not a problem
+ assertEquals("two", ThreadContext.get("inner"));
+ assertEquals("two", ThreadContext.get("outer"));
+ }
+
+ assertEquals("two", ThreadContext.get("outer"));
+ assertNull(ThreadContext.get("inner")); // This is where the test fails
+ }
+
+ assertEquals("one", ThreadContext.get("outer"));
+ assertNull(ThreadContext.get("inner"));
+ }
+ assertEquals("true", ThreadContext.get("not-in-closeable"));
+
+ assertNull(ThreadContext.get("inner"));
+ assertNull(ThreadContext.get("outer"));
+ }
}
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java
index 3f43e3e2c4..092c082ed4 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java
@@ -34,6 +34,7 @@ import java.util.Map;
import java.util.Set;
import org.apache.logging.log4j.util.ReadOnlyStringMap;
import org.apache.logging.log4j.util.TriConsumer;
+import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
public class UnmodifiableArrayBackedMapTest {
@@ -373,4 +374,23 @@ public class UnmodifiableArrayBackedMapTest {
// verify same instance, not just equals()
assertTrue(map == map.toMap());
}
+
+ @Test
+ void copyAndRemoveAll_should_work() {
+
+ // Create the actual map
+ UnmodifiableArrayBackedMap actualMap = UnmodifiableArrayBackedMap.EMPTY_MAP;
+ actualMap = actualMap.copyAndPut("outer", "two");
+ actualMap = actualMap.copyAndPut("inner", "one");
+ actualMap = actualMap.copyAndPut("not-in-closeable", "true");
+
+ // Create the expected map
+ UnmodifiableArrayBackedMap expectedMap = UnmodifiableArrayBackedMap.EMPTY_MAP;
+ expectedMap = expectedMap.copyAndPut("outer", "two");
+ expectedMap = expectedMap.copyAndPut("not-in-closeable", "true");
+
+ // Remove the key and verify
+ actualMap = actualMap.copyAndRemoveAll(Collections.singleton("inner"));
+ Assertions.assertThat(actualMap).isEqualTo(expectedMap);
+ }
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java
index eada687424..078e8c7c70 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java
@@ -350,82 +350,64 @@ public class UnmodifiableArrayBackedMap extends AbstractMap<String, String> impl
}
/**
- * Creates a new instance that contains the same entries as this map, minus all
- * of the keys passed in the arguments.
- *
- * @param key
- * @param value
- * @return
+ * Creates a new instance where the entries of provided keys are removed.
*/
public UnmodifiableArrayBackedMap copyAndRemoveAll(Iterable<String> keysToRemoveIterable) {
+
+ // Short-circuit if the map is empty
if (isEmpty()) {
- // shortcut: if this map is empty, the result will continue to be empty
return EMPTY_MAP;
}
- // now we build a Set of keys to remove
- Set<String> keysToRemoveSet;
+ // Collect distinct keys to remove
+ final Set<String> keysToRemove;
if (keysToRemoveIterable instanceof Set) {
- // we already have a set, let's cast it and reuse it
- keysToRemoveSet = (Set<String>) keysToRemoveIterable;
+ keysToRemove = (Set<String>) keysToRemoveIterable;
} else {
- // iterate through the keys and build a set
- keysToRemoveSet = new HashSet<>();
- for (String key : keysToRemoveIterable) {
- keysToRemoveSet.add(key);
+ keysToRemove = new HashSet<>();
+ for (final String key : keysToRemoveIterable) {
+ keysToRemove.add(key);
}
}
- int firstIndexToKeep = -1;
- int lastIndexToKeep = -1;
- int destinationIndex = 0;
- int numEntriesKept = 0;
- // build the new map
- UnmodifiableArrayBackedMap newMap = new UnmodifiableArrayBackedMap(numEntries);
- for (int indexInCurrentMap = 0; indexInCurrentMap < numEntries; indexInCurrentMap++) {
- // for each key in this map, check whether it's in the set we built above
- Object key = backingArray[getArrayIndexForKey(indexInCurrentMap)];
- if (!keysToRemoveSet.contains(key)) {
- // this key should be kept
- if (firstIndexToKeep == -1) {
- firstIndexToKeep = indexInCurrentMap;
- }
- lastIndexToKeep = indexInCurrentMap;
- } else if (lastIndexToKeep > 0) {
- // we hit a remove, copy any keys that are known ready
- int numEntriesToCopy = lastIndexToKeep - firstIndexToKeep + 1;
- System.arraycopy(
- backingArray,
- getArrayIndexForKey(firstIndexToKeep),
- newMap.backingArray,
- getArrayIndexForKey(destinationIndex),
- numEntriesToCopy * 2);
- firstIndexToKeep = -1;
- lastIndexToKeep = -1;
- destinationIndex += numEntriesToCopy;
- numEntriesKept += numEntriesToCopy;
- }
- }
+ // Create the new map
+ final UnmodifiableArrayBackedMap oldMap = this;
+ final int oldMapEntryCount = oldMap.numEntries;
+ final UnmodifiableArrayBackedMap newMap = new UnmodifiableArrayBackedMap(oldMapEntryCount);
- if (lastIndexToKeep > -1) {
- // at least one key still requires copying
- int numEntriesToCopy = lastIndexToKeep - firstIndexToKeep + 1;
- System.arraycopy(
- backingArray,
- getArrayIndexForKey(firstIndexToKeep),
- newMap.backingArray,
- getArrayIndexForKey(destinationIndex),
- numEntriesToCopy * 2);
- numEntriesKept += numEntriesToCopy;
+ // Short-circuit if there is nothing to remove
+ if (keysToRemove.isEmpty()) {
+ System.arraycopy(oldMap.backingArray, 0, newMap.backingArray, 0, oldMapEntryCount * 2);
+ newMap.numEntries = oldMapEntryCount;
+ return this;
}
- if (numEntriesKept == 0) {
- return EMPTY_MAP;
+ // Iterate over old map entries
+ int newMapEntryIndex = 0;
+ for (int oldMapEntryIndex = 0; oldMapEntryIndex < oldMapEntryCount; oldMapEntryIndex++) {
+ final int oldMapKeyIndex = getArrayIndexForKey(oldMapEntryIndex);
+ final Object key = oldMap.backingArray[oldMapKeyIndex];
+
+ // Skip entries of removed keys
+ @SuppressWarnings("SuspiciousMethodCalls")
+ final boolean removed = keysToRemove.contains(key);
+ if (removed) {
+ continue;
+ }
+
+ // Copy the entry
+ final int oldMapValueIndex = getArrayIndexForValue(oldMapEntryIndex);
+ final Object value = oldMap.backingArray[oldMapValueIndex];
+ final int newMapKeyIndex = getArrayIndexForKey(newMapEntryIndex);
+ final int newMapValueIndex = getArrayIndexForValue(newMapEntryIndex);
+ newMap.backingArray[newMapKeyIndex] = key;
+ newMap.backingArray[newMapValueIndex] = value;
+ newMapEntryIndex++;
}
- newMap.numEntries = numEntriesKept;
+ // Cap and return the new map
+ newMap.numEntries = newMapEntryIndex;
newMap.updateNumEntriesInArray();
-
return newMap;
}
diff --git a/src/changelog/.2.x.x/3048_fix_ThreadContext_remove.xml b/src/changelog/.2.x.x/3048_fix_ThreadContext_remove.xml
new file mode 100644
index 0000000000..d19091dc0f
--- /dev/null
+++ b/src/changelog/.2.x.x/3048_fix_ThreadContext_remove.xml
@@ -0,0 +1,8 @@
+<?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="fixed">
+ <issue id="3048" link="https://github.com/apache/logging-log4j2/pull/3048"/>
+ <description format="asciidoc">Fix key removal issues in Thread Context</description>
+</entry>
(logging-log4j2) 01/06: Bump version to `2.24.2-SNAPSHOT`
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x-site-pro
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 26538096e0132c194b89dd61ee6b28ddc0b8207b
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Fri Nov 15 22:45:52 2024 +0100
Bump version to `2.24.2-SNAPSHOT`
---
pom.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pom.xml b/pom.xml
index 40d205947e..16e79e4bb0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -306,7 +306,7 @@
<properties>
<!-- project version -->
- <revision>2.24.1</revision>
+ <revision>2.24.2-SNAPSHOT</revision>
<!-- Versions used on the site: no snapshots! -->
<site-log4j-api.version>2.24.1</site-log4j-api.version>
<site-log4j-core.version>2.24.1</site-log4j-core.version>