Skip to content

chore: Increase maximum stacktrace length to 500 #6536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −1
Open

chore: Increase maximum stacktrace length to 500 #6536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
### Improvements

- Replace deprecated SCNetworkReachability with NWPathMonitor (#6019)
- Increase number of frames per stacktrace to 500

## 8.57.0

Expand Down
2 changes: 1 addition & 1 deletion Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor_MachineContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
extern "C" {
#endif

#define MAX_STACKTRACE_LENGTH 100
#define MAX_STACKTRACE_LENGTH 500
Copy link
Member

@philipphofmann philipphofmann Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h: This has a significant impact on memory usage when allocating an array of SentryThreadInfo here

sentry-cocoa/Sources/Sentry/SentryDefaultThreadInspector.m

Line 163 in d14bd20

SentryThreadInfo threadsInfos[numSuspendedThreads];

because we allocate an array of stackEntries here

sentry-cocoa/Sources/Sentry/SentryDefaultThreadInspector.m

Lines 23 to 27 in d14bd20

typedef struct {
SentryCrashThread thread;
SentryCrashStackEntry stackEntries[MAX_STACKTRACE_LENGTH];
int stackLength;
} SentryThreadInfo;

We should evaluate what the impact is and see if it's acceptable. I'm pretty sure we can improve this for the SentryDefaultThreadInspector, so it uses less memory.

This also came up in #1923 (comment)


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Increasing MAX_STACKTRACE_LENGTH causes stack allocation to exceed background thread limits, leading to overflow.
Severity: CRITICAL | Confidence: 0.98

🔍 Detailed Analysis

Increasing MAX_STACKTRACE_LENGTH from 100 to 500 causes the SentryThreadInfo struct to grow from ~4 KB to ~20 KB per thread. When getCurrentThreadsWithStackTrace() is called on a background thread, the Variable Length Array threadsInfos allocates approximately 1.37 MB on the stack for 70 threads. This allocation exceeds the typical 512 KB stack limit for background threads, leading to a stack overflow when multiple threads are suspended and the call stack is moderately deep.

💡 Suggested Fix

Reduce MAX_STACKTRACE_LENGTH to a value that keeps total stack allocation below 512 KB for 70 threads, or refactor SentryThreadInfo allocation to use heap memory instead of the stack.

🤖 Prompt for AI Agent 
Fix this bug. In
Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor_MachineContext.h at line 36:
Increasing `MAX_STACKTRACE_LENGTH` from 100 to 500 causes the `SentryThreadInfo` struct
to grow from ~4 KB to ~20 KB per thread. When `getCurrentThreadsWithStackTrace()` is
called on a background thread, the Variable Length Array `threadsInfos` allocates
approximately 1.37 MB on the stack for 70 threads. This allocation exceeds the typical
512 KB stack limit for background threads, leading to a stack overflow when multiple
threads are suspended and the call stack is moderately deep.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: @itaybre we should try to write a test for this, maybe using a limited recursive method with parallel threads. It should be an best-effort unit test as this could potentially introduce flakiness

/** Initialize a stack cursor for a machine context.
*
Expand Down
Loading