Fix stack trace rendering for exceptions with identity malfunction

log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java

@@ -21,12 +21,16 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import foo.TestFriendlyException;
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.io.PrintStream;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -40,6 +44,7 @@
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.impl.ThrowableProxy;
 import org.apache.logging.log4j.core.layout.PatternLayout;
 import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.Nested;
@@ -571,6 +576,50 @@ private static List<Exception> createExceptionsOfDifferentDepths() {
                     })
                     .collect(Collectors.toList());
         }
+
+        @Test
+        void testThrowableProxySerializationCollision() throws Exception {
+            class CollidingException extends RuntimeException {
+                public CollidingException(String message, Throwable cause) {
+                    super(message, cause);
+                }
+
+                @Override
+                public boolean equals(Object obj) {
+                    return obj instanceof CollidingException
+                            && Objects.equals(getMessage(), ((CollidingException) obj).getMessage());
+                }
+
+                @Override
+                public int hashCode() {
+                    return Objects.hashCode(getMessage());
+                }
+            }
+
+            Throwable inner = new CollidingException("collision", null);
+            Throwable middle = new CollidingException("collision", inner);
+            Throwable outer = new CollidingException("collision", middle);
+
+            ThrowableProxy proxy = new ThrowableProxy(outer);
+
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            try (ObjectOutputStream oos = new ObjectOutputStream(baos)) {
+                oos.writeObject(proxy);
+            }
+
+            ThrowableProxy deserializedProxy;
+            try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) {
+                deserializedProxy = (ThrowableProxy) ois.readObject();
+            }
+
+            String actualStackTrace = deserializedProxy.getExtendedStackTraceAsString();
+
+            long count = Arrays.stream(actualStackTrace.split("\n"))
+                    .filter(line -> line.contains("Caused by:"))
+                    .count();
+
+            assertThat(count).isEqualTo(2);
+        }
     }
 
     static String convert(final String pattern) {

log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java

@@ -18,9 +18,10 @@
 
 import java.io.Serializable;
 import java.util.Arrays;
+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.Objects;
@@ -113,11 +114,16 @@ public ThrowableProxy(final Throwable throwable) {
         this.extendedStackTrace =
                 ThrowableProxyHelper.toExtendedStackTrace(this, stack, map, null, throwable.getStackTrace());
         final Throwable throwableCause = throwable.getCause();
-        final Set<Throwable> causeVisited = new HashSet<>(1);
+        //  `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing
+        // collisions
+        final Set<Throwable> causeVisited = Collections.newSetFromMap(new IdentityHashMap<>(1));
+        final Set<Throwable> suppressedVisited =
+                visited == null ? Collections.newSetFromMap(new IdentityHashMap<>()) : visited;
+
         this.causeProxy = throwableCause == null
                 ? null
-                : new ThrowableProxy(throwable, stack, map, throwableCause, visited, causeVisited);
-        this.suppressedProxies = ThrowableProxyHelper.toSuppressedProxies(throwable, visited);
+                : new ThrowableProxy(throwable, stack, map, throwableCause, suppressedVisited, causeVisited);
+        this.suppressedProxies = ThrowableProxyHelper.toSuppressedProxies(throwable, suppressedVisited);
     }
 
     /**

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

@@ -16,8 +16,8 @@
  */
 package org.apache.logging.log4j.core.pattern;
 
-import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -53,7 +53,10 @@ public final void renderThrowable(
         if (maxLineCount > 0) {
             try {
                 C context = createContext(throwable);
-                renderThrowable(buffer, throwable, context, new HashSet<>(), lineSeparator);
+                //  `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing
+                // collisions
+                renderThrowable(
+                        buffer, throwable, context, Collections.newSetFromMap(new IdentityHashMap<>()), lineSeparator);
             } catch (final Exception error) {
                 if (error != MAX_LINE_COUNT_EXCEEDED) {
                     throw error;
@@ -64,7 +67,11 @@ public final void renderThrowable(
 
     @SuppressWarnings("unchecked")
     C createContext(final Throwable throwable) {
-        final Map<Throwable, Context.Metadata> metadataByThrowable = Context.Metadata.ofThrowable(throwable);
+        //  `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing
+        // collisions
+        final Map<Throwable, Context.Metadata> metadataByThrowable = new IdentityHashMap<>();
+        Context.Metadata.populateMetadata(
+                metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable);
         return (C) new Context(0, metadataByThrowable);
     }
 
@@ -292,8 +299,11 @@ private Metadata(
             }
 
             static Map<Throwable, Metadata> ofThrowable(final Throwable throwable) {
-                final Map<Throwable, Metadata> metadataByThrowable = new HashMap<>();
-                populateMetadata(metadataByThrowable, new HashSet<>(), null, throwable);
+                //  `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing
+                // collisions
+                final Map<Throwable, Metadata> metadataByThrowable = new IdentityHashMap<>();
+                populateMetadata(
+                        metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable);
                 return metadataByThrowable;
             }
 

src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml

@@ -0,0 +1,13 @@
+<?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="changed">
+    <issue id="3933" link="https://github.com/apache/logging-log4j2/issues/3933"/>
+    <issue id="4133" link="https://github.com/apache/logging-log4j2/pull/4133"/>
+    <description format="asciidoc">
+        Fix `circular reference` detection for exceptions with `colliding` equals/hashCode implementations
+    </description>
+</entry>