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 2026/08/12 08:37:53 UTC
(logging-log4j2) 01/01: Fix circular references in exceptions
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch stack-trace-fix
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit bd6a7e6c0a6a4151cd7b816cfc23e7e2916b613c
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Wed Aug 12 10:37:13 2026 +0200
Fix circular references in exceptions
---
.../appender/nosql/NoSqlDatabaseManagerTest.java | 25 ++++++++++++++++++++++
.../logging/log4j/core/util/ThrowablesTest.java | 9 ++++++++
.../core/appender/nosql/NoSqlDatabaseManager.java | 10 +++++++--
.../ThrowableExtendedStackTraceRenderer.java | 4 ++--
.../apache/logging/log4j/core/util/Throwables.java | 5 +++--
.../jpa/converter/ThrowableAttributeConverter.java | 20 +++++++++++------
.../converter/ThrowableAttributeConverterTest.java | 14 ++++++++++++
.../.2.x.x/4248_fix-circular-exception.xml | 12 +++++++++++
8 files changed, 87 insertions(+), 12 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManagerTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManagerTest.java
index e9c53a3706..d53769e8b8 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManagerTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManagerTest.java
@@ -25,6 +25,7 @@ import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.then;
import static org.mockito.Mockito.mock;
+import foo.TestFriendlyException;
import java.io.IOException;
import java.sql.SQLException;
import java.util.Collection;
@@ -428,4 +429,28 @@ public class NoSqlDatabaseManagerTest {
assertEquals("The context stack is not correct.", stack.asList(), object.get("contextStack"));
}
}
+
+ @Test
+ public void testWriteInternalWithCyclicCause() {
+ given(connection.isClosed()).willReturn(false);
+ final Throwable exception = TestFriendlyException.INSTANCE;
+
+ try (final NoSqlDatabaseManager<?> manager =
+ NoSqlDatabaseManager.getNoSqlDatabaseManager("name", 0, provider, null, null)) {
+ manager.startup();
+ manager.connectAndStart();
+ manager.writeInternal(
+ Log4jLogEvent.newBuilder().setThrown(exception).build(), null);
+
+ then(connection).should().insertObject(captor.capture());
+ final Map<String, Object> thrown =
+ (Map<String, Object>) captor.getValue().unwrap().get("thrown");
+ final Map<String, Object> cause = (Map<String, Object>) thrown.get("cause");
+ final Map<String, Object> nestedCause = (Map<String, Object>) cause.get("cause");
+ assertEquals(exception.getMessage(), thrown.get("message"));
+ assertEquals(exception.getCause().getMessage(), cause.get("message"));
+ assertEquals(exception.getCause().getCause().getMessage(), nestedCause.get("message"));
+ assertNull("The cycle should not be serialized.", nestedCause.get("cause"));
+ }
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/ThrowablesTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/ThrowablesTest.java
index 912d62a1d9..b1a2370aae 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/ThrowablesTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/ThrowablesTest.java
@@ -17,8 +17,10 @@
package org.apache.logging.log4j.core.util;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import foo.TestFriendlyException;
import org.junit.jupiter.api.Test;
class ThrowablesTest {
@@ -53,6 +55,13 @@ class ThrowablesTest {
assertEquals(cause1, Throwables.getRootCause(cause3));
}
+ @Test
+ void testGetRootCauseWithCollidingExceptions() {
+ final Throwable throwable = TestFriendlyException.INSTANCE;
+ final Throwable rootCause = throwable.getCause().getCause();
+ assertSame(rootCause, Throwables.getRootCause(throwable));
+ }
+
@Test
void testRethrowRuntimeException() {
assertThrows(NullPointerException.class, () -> Throwables.rethrow(new NullPointerException()));
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java
index e053944a0c..edfa07178c 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java
@@ -17,7 +17,10 @@
package org.apache.logging.log4j.core.appender.nosql;
import java.io.Serializable;
+import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.Objects;
+import java.util.Set;
import java.util.stream.Stream;
import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.ThreadContext;
@@ -223,8 +226,11 @@ public final class NoSqlDatabaseManager<W> extends AbstractDatabaseManager {
exceptionEntity.set("type", thrown.getClass().getName());
exceptionEntity.set("message", thrown.getMessage());
exceptionEntity.set("stackTrace", this.convertStackTrace(thrown.getStackTrace()));
- while (thrown.getCause() != null) {
- thrown = thrown.getCause();
+ final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
+ visitedThrowables.add(thrown);
+ Throwable cause;
+ while ((cause = thrown.getCause()) != null && visitedThrowables.add(cause)) {
+ thrown = cause;
final NoSqlObject<W> causingExceptionEntity = this.connection.createObject();
causingExceptionEntity.set("type", thrown.getClass().getName());
causingExceptionEntity.set("message", thrown.getMessage());
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableExtendedStackTraceRenderer.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableExtendedStackTraceRenderer.java
index c70e57d4d4..114bea94f5 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableExtendedStackTraceRenderer.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableExtendedStackTraceRenderer.java
@@ -20,7 +20,7 @@ import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
-import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Queue;
@@ -112,7 +112,7 @@ final class ThrowableExtendedStackTraceRenderer
final Map<String, ClassResourceInfo> classResourceInfoByName = new HashMap<>();
// Walk over the causal chain
- final Set<Throwable> visitedThrowables = new HashSet<>();
+ final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
final Queue<Throwable> pendingThrowables = new ArrayDeque<>(Collections.singleton(rootThrowable));
Throwable throwable;
while ((throwable = pendingThrowables.poll()) != null && visitedThrowables.add(throwable)) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Throwables.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Throwables.java
index cf00631f9f..a673000674 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Throwables.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Throwables.java
@@ -25,7 +25,8 @@ import java.io.PrintWriter;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Set;
import org.apache.logging.log4j.core.internal.annotation.SuppressFBWarnings;
@@ -46,7 +47,7 @@ public final class Throwables {
*/
public static Throwable getRootCause(final Throwable throwable) {
requireNonNull(throwable, "throwable");
- final Set<Throwable> visitedThrowables = new HashSet<>();
+ final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
Throwable prevCause = throwable;
visitedThrowables.add(prevCause);
Throwable nextCause;
diff --git a/log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java b/log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java
index 693d05dcf2..1448d3b287 100644
--- a/log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java
+++ b/log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java
@@ -20,8 +20,11 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.ListIterator;
+import java.util.Set;
import javax.persistence.AttributeConverter;
import javax.persistence.Converter;
import org.apache.logging.log4j.util.LoaderUtil;
@@ -64,13 +67,18 @@ public class ThrowableAttributeConverter implements AttributeConverter<Throwable
}
private void convertThrowable(final StringBuilder builder, final Throwable throwable) {
- builder.append(throwable.toString()).append('\n');
- for (final StackTraceElement element : throwable.getStackTrace()) {
- builder.append("\tat ").append(element).append('\n');
- }
- if (throwable.getCause() != null) {
+ final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
+ for (Throwable currentThrowable = throwable; visitedThrowables.add(currentThrowable); ) {
+ builder.append(currentThrowable).append('\n');
+ for (final StackTraceElement element : currentThrowable.getStackTrace()) {
+ builder.append("\tat ").append(element).append('\n');
+ }
+ final Throwable cause = currentThrowable.getCause();
+ if (cause == null || visitedThrowables.contains(cause)) {
+ break;
+ }
builder.append("Caused by ");
- this.convertThrowable(builder, throwable.getCause());
+ currentThrowable = cause;
}
}
diff --git a/log4j-jpa/src/test/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverterTest.java b/log4j-jpa/src/test/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverterTest.java
index f57d1762ed..5ad40fd144 100644
--- a/log4j-jpa/src/test/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverterTest.java
+++ b/log4j-jpa/src/test/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverterTest.java
@@ -19,8 +19,10 @@ package org.apache.logging.log4j.core.appender.db.jpa.converter;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.sql.SQLException;
+import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
@@ -70,6 +72,18 @@ class ThrowableAttributeConverterTest {
assertEquals(stackTrace, getStackTrace(reversed), "The reversed value is not correct.");
}
+ @Test
+ void testConvertCyclicCause() {
+ final Exception exception1 = new Exception("exception1");
+ final Exception exception2 = new Exception("exception2");
+ exception1.initCause(exception2);
+ exception2.initCause(exception1);
+ final String converted = converter.convertToDatabaseColumn(exception1);
+ assertTrue(converted.contains("exception1"));
+ assertTrue(converted.contains("exception2"));
+ assertEquals(1, StringUtils.countMatches(converted, "Caused by "));
+ }
+
@Test
void testConvertNullToDatabaseColumn() {
assertNull(this.converter.convertToDatabaseColumn(null), "The converted value should be null.");
diff --git a/src/changelog/.2.x.x/4248_fix-circular-exception.xml b/src/changelog/.2.x.x/4248_fix-circular-exception.xml
new file mode 100644
index 0000000000..369b5429fe
--- /dev/null
+++ b/src/changelog/.2.x.x/4248_fix-circular-exception.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="4248" link="https://github.com/apache/logging-log4j2/pull/4248"/>
+ <description format="asciidoc">
+ Fix handling of circular references in exceptions
+ </description>
+</entry>