Increase the maximum stacktrace frame length

[brustolin]

Description

We are using 100 as a constant to create C arrays of Stacktrace frames. The App hangs PR #1906 do this in a context that is not possible to allocate more memory. We already received a few complaints that the Stacktrace is truncated and this may be the cause.

The first idea to improve this is to increase the value, with the obvious downside that we need to pre-allocate more memory every time we need to create the stack trace.

[philipphofmann]

@Swatinem and @armcknight , can you maybe help us here?

We allocate a struct with a maximum of 100 stack entries before suspending the threads. Here we are walking the stack cursor after suspending the threads. Do you have any recommendations on what to do when the stack has more than 100 entries? Now we ignore this, and the stacktrace would miss a few entries. We can't allocate memory either.

https://github.com/getsentry/sentry-cocoa/blob/02de834216db8870d340851f5aad219ceae3d167/Sources/Sentry/SentryThreadInspector.m#L114-L134

[Swatinem]

I can’t think of anything, except increase the maximum length ;-) We have the same limitations in native, and there we reserve space for 256 frames.

[philipphofmann]

Thanks, @Swatinem, for your reply. I thought so, but worth a shot. @brustolin, I think we could increase the value to 256 then.

[philipphofmann]

Do you do anything special when you exceed the maximum, like adding a special entry or making sure you cut from the bottom or top, or do you just ignore it, @Swatinem?

[Swatinem]

It just truncates at the bottom and stops unwinding when it reaches the limit.

[armcknight]

An alternative approach is to not keep the stack trace in memory, and write it out to disk (it can be deserialized back into memory outside of the thread-suspended sector). This is how Crashlytics does it: https://github.com/firebase/firebase-ios-sdk/blob/e2542c7932b0277bb4f5c6a9e0affa7076065481/Crashlytics/Crashlytics/Components/FIRCLSProcess.c#L389

They record a maximum of 600,000 frames on iOS: https://github.com/firebase/firebase-ios-sdk/blob/e2542c7932b0277bb4f5c6a9e0affa7076065481/Crashlytics/Crashlytics/Unwind/FIRCLSUnwind.c#L26-L34

This has an additional bonus that some data is preserved if for whatever reason we are terminated while doing this work. But, it comes at a cost of some additional implementation, and working with fds and write ain't that fun 🙃

[philipphofmann]

We also write stacktrace to disk when we crash, but this one we use when not crashing. So for App Hangs and in the future when calling captureMessage or captureError. Writing to disk when not crashing is definitely a tradeoff, which might not be worth it.

[brustolin]

Since this is only used for ANR, we are fine with increasing the maximum stack frame size to 256.

[philipphofmann]

For most SentryCrash monitors we use SentryCrashSC_STACK_OVERFLOW_THRESHOLD

https://github.com/getsentry/sentry-cocoa/blob/e65226c90c1bb83ab1c40483c8e310d686e1f11f/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h#L42

and for some others we use MAX_STACKTRACE_LENGTH

https://github.com/getsentry/sentry-cocoa/blob/e65226c90c1bb83ab1c40483c8e310d686e1f11f/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor_MachineContext.h#L34

We advance the cursor while writing a report to disk in here https://github.com/getsentry/sentry-cocoa/blob/e65226c90c1bb83ab1c40483c8e310d686e1f11f/Sources/SentryCrash/Recording/SentryCrashReport.c#L816-L850

So I think for hard crashes we could increase the limit easily, but that's not the purpose of this issue. FYI, @armcknight.

[philipphofmann]

Just to be safe, we could only increase the stacktrace size for App Hangs to avoid possible side effects.

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀