Skip to content

Fix NullPointerException in ThrowableStackTraceRenderer for non-idempotent getCause() - #4303

Open
UsmanEjaz10 wants to merge 6 commits into
apache:2.xfrom
UsmanEjaz10:issue-4279-fix
Open

Fix NullPointerException in ThrowableStackTraceRenderer for non-idempotent getCause()#4303
UsmanEjaz10 wants to merge 6 commits into
apache:2.xfrom
UsmanEjaz10:issue-4279-fix

Conversation

@UsmanEjaz10

Copy link
Copy Markdown

What & why

Rendering a Throwable walks its causal chain in two passes: a metadata pre-computation pass and a render pass. The render pass called Throwable#getCause() a second time after the metadata pass had keyed the first result in its IdentityHashMap. For a getCause() override that returns a different instance on every call (legal per the Throwable API contract), the second lookup misses, and rendering dereferences a null Metadata, throwing:

java.lang.NullPointerException: Cannot read field "stackLength" because "metadata" is null
    at org.apache.logging.log4j.core.pattern.ThrowableStackTraceRenderer.renderStackTraceElements(...)

Fix

Capture the cause once in Context.Metadata during the metadata pass and reuse metadata.cause at render time — in the base renderer (%ex, and %xEx which inherits it) and in the inverted renderer (%rEx). This is the same treatment the class already applies to getStackTrace() (#3940) and getSuppressed() (#3929).

ThrowableExtendedStackTraceRenderer is intentionally left alone: its independent JAR-enrichment walk null-checks and degrades gracefully, and it is a legitimate second getCause() caller — so %xEx invokes getCause() twice by design.

Tests

New NonIdempotentGetCauseTest asserts the fix across all three converters:

  • %ex / %rEx: getCause() invoked exactly once per render.
  • %xEx: invoked exactly twice (its own enrichment walk is the second, legitimate call).
  • A non-idempotent getCause() renders its cause without throwing.

Kept as a standalone class rather than added to the shared AbstractStackTraceTest, which mutates a static EXCEPTION concurrently and makes adding test methods there unsafe w.r.t. execution order.

Verification

  • ./mvnw verify (scoped to log4j-core, log4j-core-test and upstream modules): passes — RAT, Spotless, japicmp all green.
  • Tests: 1,398 across the four affected test classes, 0 failures.
  • Changelog entry added under src/changelog/.2.x.x/.

Fixes #4279

…dempotent `getCause()` (apache#4279)

Rendering walked the causal chain twice. The render pass called
`Throwable#getCause()` a second time after the metadata pre-computation pass
had keyed the first result in its `IdentityHashMap`. A non-idempotent
`getCause()` returns a different instance on that second call, the lookup
misses, and rendering dereferences a null `Metadata` (`stackLength` NPE).

Capture the cause once in `Context.Metadata` during the metadata pass and
reuse `metadata.cause` at render time, in both the base and inverted
renderers - the same treatment already applied to `getStackTrace()` (apache#3940)
and `getSuppressed()` (apache#3929). The extended renderer (`%xEx`) keeps its own
JAR-enrichment walk, which null-checks and is a legitimate second `getCause()`
caller.

Adds `NonIdempotentGetCauseTest` covering `%ex`, `%rEx` (invoked once) and
`%xEx` (invoked twice).

Signed-off-by: UsmanEjaz10 <[email protected]>

Co-Authored-By: Claude Code <[email protected]>
@ramanathan1504

Copy link
Copy Markdown
Contributor

@UsmanEjaz10 Noted!. Give some time i will review it. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

ThrowableStackTraceRenderer throws NullPointerException when Throwable.getCause() returns different instances

2 participants