Fix stack trace rendering for exceptions with identity malfunction

log4j-core-test/src/test/java/foo/TestFriendlyException.java

@@ -20,6 +20,12 @@
 
 import java.net.Socket;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.LinkedList;
+import java.util.Queue;
+import java.util.Set;
 import java.util.stream.Stream;
 import org.apache.logging.log4j.util.Constants;
 
@@ -33,6 +39,7 @@
  * <li>Suppressed exceptions</li>
  * <li>Clutter-free stack trace (i.e., elements from JUnit, JDK, etc.)</li>
  * <li>Stack trace elements from named modules<sup>3</sup></li>
+ * <li>Exceptions with malfunctioning (e.g., colliding) {@link Object#equals(Object) equals()} and {@link Object#hashCode() hashCode()} implementations in the causal chain</li>
  * </ul>
  * <p>
  * <sup>1</sup> Helps with observing stack trace manipulation effects of Log4j.
@@ -80,20 +87,63 @@ private static StackTraceElement namedModuleStackTraceElement() {
         "java.lang", "jdk.internal", "org.junit", "sun.reflect"
     };
 
-    public static final TestFriendlyException INSTANCE = create("r", 0, 2, new boolean[] {false}, new boolean[] {true});
+    public static final TestFriendlyException INSTANCE =
+            create("r", 0, 2, new boolean[] {false}, new boolean[] {true}, new int[] {5});
+
+    static {
+        ensureIdentityMalfunctionAtDifferentDepths();
+    }
+
+    /**
+     * Ensure we have identity malfunctioning exceptions that have different stack trace lengths.
+     *
+     * @see <a href="https://github.com/apache/logging-log4j2/issues/3933">#3933</a>
+     */
+    private static void ensureIdentityMalfunctionAtDifferentDepths() {
+        final Set<Throwable> visitedExceptions = Collections.newSetFromMap(new IdentityHashMap<>());
+        final Set<Integer> identityMalfunctioningExceptionStackTraceDepths = new HashSet<>();
+        final Queue<TestFriendlyException> exceptions = new LinkedList<>();
+        exceptions.add(INSTANCE);
+        while (!exceptions.isEmpty()) {
+
+            // Process the exception
+            final TestFriendlyException exception = exceptions.remove();
+            if (!visitedExceptions.add(exception) || !exception.identityMalfunctioning) {
+                continue;
+            }
+            identityMalfunctioningExceptionStackTraceDepths.add(exception.getStackTrace().length);
+
+            // Enqueue the cause
+            final TestFriendlyException cause = (TestFriendlyException) exception.getCause();
+            if (cause != null) {
+                exceptions.add(cause);
+            }
+
+            // Enqueue the suppressed
+            for (final Throwable suppressed : exception.getSuppressed()) {
+                exceptions.add((TestFriendlyException) suppressed);
+            }
+        }
+        assertThat(identityMalfunctioningExceptionStackTraceDepths)
+                .describedAs("# of visited exceptions = %s", visitedExceptions.size())
+                .hasSizeGreaterThan(1);
+    }
 
     private static TestFriendlyException create(
             final String name,
             final int depth,
             final int maxDepth,
             final boolean[] circular,
-            final boolean[] namedModuleAllowed) {
-        final TestFriendlyException error = new TestFriendlyException(name, namedModuleAllowed);
+            final boolean[] namedModuleAllowed,
+            final int[] maxIdentityMalfunctionCount) {
+        final TestFriendlyException error =
+                new TestFriendlyException(name, namedModuleAllowed, maxIdentityMalfunctionCount);
         if (depth < maxDepth) {
-            final TestFriendlyException cause = create(name + "_c", depth + 1, maxDepth, circular, namedModuleAllowed);
+            final TestFriendlyException cause =
+                    create(name + "_c", depth + 1, maxDepth, circular, namedModuleAllowed, maxIdentityMalfunctionCount);
             error.initCause(cause);
             final TestFriendlyException suppressed =
-                    create(name + "_s", depth + 1, maxDepth, circular, namedModuleAllowed);
+                    create(name + "_s", depth + 1, maxDepth, circular, namedModuleAllowed, maxIdentityMalfunctionCount);
             error.addSuppressed(suppressed);
             final boolean circularAllowed = depth + 1 == maxDepth && !circular[0];
             if (circularAllowed) {
@@ -105,8 +155,12 @@ private static TestFriendlyException create(
         return error;
     }
 
-    private TestFriendlyException(final String message, final boolean[] namedModuleAllowed) {
+    private final boolean identityMalfunctioning;
+
+    private TestFriendlyException(
+            final String message, final boolean[] namedModuleAllowed, final int[] maxIdentityMalfunctionCount) {
         super(message);
+        this.identityMalfunctioning = --maxIdentityMalfunctionCount[0] > 0;
         removeExcludedStackTraceElements(namedModuleAllowed);
     }
 
@@ -171,4 +225,14 @@ private static Stream<StackTraceElement> namedModuleIncludedStackTraceElement(fi
     public String getLocalizedMessage() {
         return getMessage() + " [localized]";
     }
+
+    @Override
+    public int hashCode() {
+        return identityMalfunctioning ? 0 : super.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        return identityMalfunctioning || super.equals(obj);
+    }
 }

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 for exceptions with identity malfunction.
+        // Consider `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 for exceptions with identity malfunction.
+                // Consider `equals()` and `hashCode()` implementations causing collisions.
+                final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
+                renderThrowable(buffer, throwable, context, visitedThrowables, lineSeparator);
             } catch (final Exception error) {
                 if (error != MAX_LINE_COUNT_EXCEEDED) {
                     throw error;
@@ -292,8 +295,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 for exceptions with identity malfunction.
+                // Consider `equals()` and `hashCode()` implementations causing collisions.
+                final Map<Throwable, Metadata> metadataByThrowable = new IdentityHashMap<>();
+                final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>());
+                populateMetadata(metadataByThrowable, visitedThrowables, 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 stack trace rendering for exceptions with identity malfunction (e.g., colliding `equals()` and/or `hashCode()` implementations)
+  </description>
+</entry>