This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.x by this push:
new 2c66a74fd7 fix: Prevent `LogBuilder` memory leak in Log4j API to
Logback bridge (#3824)
2c66a74fd7 is described below
commit 2c66a74fd712976b2111bad6719d6649553b0cbf
Author: Piotr P. Karwasz <pkarwasz-git...@apache.org>
AuthorDate: Sun Jul 20 20:01:15 2025 +0200
fix: Prevent `LogBuilder` memory leak in Log4j API to Logback bridge (#3824)
---
log4j-to-slf4j/pom.xml | 39 ++++++++++++++++++++
.../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, 103 insertions(+), 7 deletions(-)
diff --git a/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml
index 3a6b0d9e85..a2d6f0a664 100644
--- a/log4j-to-slf4j/pom.xml
+++ b/log4j-to-slf4j/pom.xml
@@ -153,6 +153,45 @@
</execution>
</executions>
</plugin>
+
</plugins>
</build>
+
+ <profiles>
+
+ <!-- Fixes incompatible with Java 8 -->
+ <profile>
+
+ <id>java8-incompat-fixes</id>
+
+ <!-- CI uses Java 8 for running tests.
+ Hence, we assume CI=Java8 and apply our changes elsewhere.
+
+ One might think why not activate using `<jdk>[16,)` instead?
+ This doesn't work, since the match is not against "the JDK running
tests", but "the JDK running Maven".
+ These two JDKs can differ due to Maven Toolchains.
+ See `java8-tests` profile in `/pom.xml` for details. -->
+ <activation>
+ <property>
+ <name>!env.CI</name>
+ </property>
+ </activation>
+
+ <!-- Illegal access is disabled by default in Java 16 due to JEP-396.
+ We are relaxing it for tests. -->
+ <build>
+ <plugins>
+ <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>
+
+ </profile>
+
+ </profiles>
</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>
