This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch fix/2.x/3819-logback-builder-reuse
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 05d97d2ac1ed51b071541d53b7e18b8852d4c18a
Author: Piotr P. Karwasz <pkarwasz-git...@apache.org>
AuthorDate: Mon Jul 14 13:21:24 2025 +0200
fix: Prevent `LogBuilder` memory leak in Log4j API to Logback bridge
This change fixes a potential memory leak in the `log4j-to-slf4j` module
(Log4j API to Logback bridge) caused by the use of a thread-local `LogBuilder`
pool.
The leak occurred because the thread-local field was accessed, even when
`log4j2.enableThreadlocals` was set to `false`. In servlet environments, this
could lead to classloader leaks due to the persistence of thread-local
references.
With this fix, all access to the `ThreadLocal` is now **properly** gated by
the `log4j2.enableThreadlocals` system property—matching how similar pooling
behavior is handled in Log4j Core.
This preserves the GC-free option for advanced users who explicitly opt in
via `log4j2.enableThreadlocals = true`, while avoiding memory leaks for others.
Fixes #3819
---
log4j-to-slf4j/pom.xml | 11 ++++++
.../java/org/apache/logging/slf4j/SLF4JLogger.java | 16 +++++---
.../{LoggerTest.java => SLF4JLoggerTest.java} | 43 +++++++++++++++++++++-
.../slf4j/{LoggerTest.xml => SLF4JLoggerTest.xml} | 0
.../.2.x.x/3819_logback-builder-reuse.xml | 12 ++++++
5 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml
index 3a6b0d9e85..95e3ec5fc9 100644
--- a/log4j-to-slf4j/pom.xml
+++ b/log4j-to-slf4j/pom.xml
@@ -153,6 +153,17 @@
</execution>
</executions>
</plugin>
+
+ <!-- Illegal access is disabled by default in Java 16 due to JEP-396.
+ We are relaxing it for tests. -->
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
+ </configuration>
+ </plugin>
+
</plugins>
</build>
</project>
diff --git
a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
index 26e94c67b3..ff9410f33e 100644
--- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
+++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
@@ -42,17 +42,21 @@ public class SLF4JLogger extends AbstractLogger {
private final org.slf4j.Logger logger;
private final LocationAwareLogger locationAwareLogger;
+ private final boolean useThreadLocal;
public SLF4JLogger(final String name, final MessageFactory messageFactory,
final org.slf4j.Logger logger) {
- super(name, messageFactory);
- this.logger = logger;
- this.locationAwareLogger = logger instanceof LocationAwareLogger ?
(LocationAwareLogger) logger : null;
+ this(name, messageFactory, logger, Constants.ENABLE_THREADLOCALS);
}
public SLF4JLogger(final String name, final org.slf4j.Logger logger) {
- super(name);
+ this(name, null, logger);
+ }
+
+ SLF4JLogger(String name, MessageFactory messageFactory, org.slf4j.Logger
logger, boolean useThreadLocal) {
+ super(name, messageFactory);
this.logger = logger;
this.locationAwareLogger = logger instanceof LocationAwareLogger ?
(LocationAwareLogger) logger : null;
+ this.useThreadLocal = useThreadLocal;
}
private int convertLevel(final Level level) {
@@ -364,8 +368,8 @@ public class SLF4JLogger extends AbstractLogger {
@Override
protected LogBuilder getLogBuilder(final Level level) {
- final SLF4JLogBuilder builder = logBuilder.get();
- return Constants.ENABLE_THREADLOCALS && !builder.isInUse()
+ SLF4JLogBuilder builder;
+ return useThreadLocal && !(builder = logBuilder.get()).isInUse()
? builder.reset(this, level)
: new SLF4JLogBuilder(this, level);
}
diff --git
a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/SLF4JLoggerTest.java
similarity index 82%
rename from
log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
rename to
log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/SLF4JLoggerTest.java
index 6c378ad958..9e3a5856a0 100644
--- a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
+++ b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/SLF4JLoggerTest.java
@@ -32,10 +32,13 @@ import static org.junit.jupiter.api.Assertions.fail;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.testUtil.StringListAppender;
+import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Date;
import java.util.List;
+import org.apache.logging.log4j.LogBuilder;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.ThreadContext;
@@ -45,13 +48,17 @@ import
org.apache.logging.log4j.message.StringFormatterMessageFactory;
import org.apache.logging.log4j.spi.AbstractLogger;
import org.apache.logging.log4j.spi.MessageFactory2Adapter;
import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.junitpioneer.jupiter.Issue;
import org.slf4j.MDC;
@UsingStatusListener
@LoggerContextSource
-class LoggerTest {
+class SLF4JLoggerTest {
private static final Object OBJ = new Object();
// Log4j objects
@@ -265,4 +272,38 @@ class LoggerTest {
// expected
}
}
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ @Issue("https://github.com/apache/logging-log4j2/issues/3819";)
+ void threadLocalUsage(boolean useThreadLocal) throws
ReflectiveOperationException {
+ // Reset the static ThreadLocal in SLF4JLogger
+ getLogBuilderThreadLocal().remove();
+ final org.slf4j.Logger slf4jLogger = context.getLogger(getClass());
+ Logger logger = new SLF4JLogger(slf4jLogger.getName(), null,
slf4jLogger, useThreadLocal);
+ LogBuilder builder1 = logger.atInfo();
+ builder1.log("Test message");
+ LogBuilder builder2 = logger.atInfo();
+ builder2.log("Another test message");
+ // Check if the same builder is reused based on the useThreadLocal flag
+ Assertions.assertThat(isThreadLocalPresent())
+ .as("ThreadLocal should be present iff useThreadLocal is
enabled")
+ .isEqualTo(useThreadLocal);
+ Assertions.assertThat(builder2 == builder1)
+ .as("Builder2 should be the same as Builder1 iff
useThreadLocal is enabled")
+ .isEqualTo(useThreadLocal);
+ }
+
+ private static boolean isThreadLocalPresent() throws
ReflectiveOperationException {
+ Method isPresentMethod =
ThreadLocal.class.getDeclaredMethod("isPresent");
+ isPresentMethod.setAccessible(true);
+ ThreadLocal<?> threadLocal = getLogBuilderThreadLocal();
+ return (boolean) isPresentMethod.invoke(threadLocal);
+ }
+
+ private static ThreadLocal<?> getLogBuilderThreadLocal() throws
ReflectiveOperationException {
+ Field logBuilderField =
SLF4JLogger.class.getDeclaredField("logBuilder");
+ logBuilderField.setAccessible(true);
+ return (ThreadLocal<?>) logBuilderField.get(null);
+ }
}
diff --git
a/log4j-to-slf4j/src/test/resources/org/apache/logging/slf4j/LoggerTest.xml
b/log4j-to-slf4j/src/test/resources/org/apache/logging/slf4j/SLF4JLoggerTest.xml
similarity index 100%
rename from
log4j-to-slf4j/src/test/resources/org/apache/logging/slf4j/LoggerTest.xml
rename to
log4j-to-slf4j/src/test/resources/org/apache/logging/slf4j/SLF4JLoggerTest.xml
diff --git a/src/changelog/.2.x.x/3819_logback-builder-reuse.xml
b/src/changelog/.2.x.x/3819_logback-builder-reuse.xml
new file mode 100644
index 0000000000..ea67aa50d0
--- /dev/null
+++ b/src/changelog/.2.x.x/3819_logback-builder-reuse.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry 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";
+ type="fixed">
+ <issue id="3819"
link="https://github.com/apache/logging-log4j2/issues/3819"/>
+ <description format="asciidoc">
+ Fix potential memory leak involving `LogBuilder` in Log4j API to Logback
bridge.
+ </description>
+</entry>
