Fix circular references in exceptions

log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManagerTest.java

@@ -25,6 +25,7 @@
 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 void testWriteInternal03() {
             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"));
+        }
+    }
 }

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 @@ void testGetRootCauseLoop() {
         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()));

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 @@ private void setFields(final LogEvent event, final NoSqlObject<W> entity) {
             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());

log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableExtendedStackTraceRenderer.java

@@ -20,7 +20,7 @@
 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 @@ private static Map<String, ClassResourceInfo> createClassResourceInfoByName(
             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)) {

log4j-core/src/main/java/org/apache/logging/log4j/core/util/Throwables.java

@@ -25,7 +25,8 @@
 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 @@ private 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;

log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java

@@ -20,8 +20,11 @@
 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 String convertToDatabaseColumn(final Throwable 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;
         }
     }
 

log4j-jpa/src/test/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverterTest.java

@@ -19,8 +19,10 @@
 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 @@ void testConvert02() {
         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.");

src/changelog/.2.x.x/4249_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="4249" link="https://github.com/apache/logging-log4j2/pull/4249"/>
+    <description format="asciidoc">
+        Fix `Throwable` causal-chain handling for cyclic and identity-malfunctioning exceptions
+    </description>
+</entry>