Repository navigation
Fix uncaught StackOverflowError in StackTraces.getStackTrace - #12233
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
Conversation
printStackTrace() can itself throw a second StackOverflowError when formatting a throwable caught with little remaining stack margin. The existing fallback only caught Exception, so the second StackOverflowError (an Error) escaped uncaught. Widen the catches to Throwable so it falls through to the manual getStackTrace()-based reconstruction instead. APMLP-1767 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Widen safeGetMessage's catch to Throwable, and try t.getMessage() in the innermost fallback of StackTraces.getStackTrace before giving up on just the class name, so a bit more diagnostic context survives even when both printStackTrace() and getStackTrace() throw. APMLP-1767 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cf9c0427c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
A recovered long stack trace still enters a final truncation block that catches only Exception, allowing another StackOverflowError to escape under the same low-stack condition this change targets.
🤖 Datadog Autotest · Commit 4cf9c04 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
catch (Throwable) also swallowed ThreadDeath and unrelated fatal Errors; narrow to the two cases this fix actually needs to recover from. Also extend the same recovery to the truncate() call so a second StackOverflowError while the stack is still constrained during truncation doesn't escape uncaught. Addresses codex review comments on #12233.
There was a problem hiding this comment.
More details
The code now catches StackOverflowError at each stack trace formatting step. Other Error types still pass to the caller.
🤖 Datadog Autotest · Commit f6a2f4a · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
c162404
into
master
What Does This Do
Widens the failure-handling in
StackTraces.safeGetMessageandStackTraces.getStackTraceto also catchStackOverflowError, and adds a deeper fallback chain ingetStackTracefor when even thegetStackTrace()-array reconstruction fails.Motivation
Intended to improve the case where a
StackOverflowErroris being reported. In that situation,StackTraces.getStackTraceis likely to trigger a freshStackOverflowError.Additional Notes
StackTraces.getStackTracecaughtprintStackTrace()failures withcatch (Exception ...), so a secondStackOverflowErrorthrown while formatting an already stack-constrained throwable escaped uncaught instead of falling back to the manualgetStackTrace()-based reconstruction.safeGetMessageandgetStackTracetocatch (Exception | StackOverflowError ...)— deliberately not the broaderThrowable, so otherErrors (e.g.OutOfMemoryError) still propagate instead of being silently swallowed.getStackTrace: if thegetStackTrace()-array reconstruction itself throws, fall back to<class name>: <message>, and if even that throws, fall back to just the class name.03773be6-9b34-11f1-930e-da7ad0900002(serviceci360-engage-optimize): aStackOverflowErrorin the customer's app was caught by our instrumentation with almost no stack margin left, and capturing its trace triggered a second overflow that propagated out of our error-handling code.Jira: APMLP-1767
Test plan
TestThrowables.throwingStackOverflowOnPrintStackTrace(),throwingStackOverflowEverywhereExceptGetMessage(), andthrowingStackOverflowEverywhere(), with unit tests assertinggetStackTracefalls back gracefully at each level instead of throwing./gradlew :dd-trace-core:test --tests "datadog.trace.core.util.StackTracesTest"passes./gradlew :dd-trace-core:spotlessApply🤖 Generated with Claude Code