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.

sentry-cocoa/Sources/Sentry/SentryThreadInspector.m

Lines 114 to 134 in 02de834

	sentrycrashmc_suspendEnvironment(&suspendedThreads, &numSuspendedThreads);
	// DANGER: Do not try to allocate memory in the heap or call Objective-C code in this
	// section Doing so when the threads are suspended may lead to deadlocks or crashes.
	SentryThreadInfo threadsInfos[numSuspendedThreads];
	for (int i = 0; i < numSuspendedThreads; i++) {
	if (suspendedThreads[i] != currentThread) {
	int numberOfEntries = getStackEntriesFromThread(suspendedThreads[i], context,
	threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH);
	threadsInfos[i].stackLength = numberOfEntries;
	} else {
	// We can't use 'getStackEntriesFromThread' to retrieve stack frames from the
	// current thread. We are using the stackTraceBuilder to retrieve this information
	// later.
	threadsInfos[i].stackLength = 0;
	}
	threadsInfos[i].thread = suspendedThreads[i];
	}
	sentrycrashmc_resumeEnvironment(suspendedThreads, numSuspendedThreads);

[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

sentry-cocoa/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h

Line 42 in e65226c

#define SentryCrashSC_STACK_OVERFLOW_THRESHOLD 150

and for some others we use MAX_STACKTRACE_LENGTH

sentry-cocoa/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor_MachineContext.h

Line 34 in e65226c

#define MAX_STACKTRACE_LENGTH 100

We advance the cursor while writing a report to disk in here

sentry-cocoa/Sources/SentryCrash/Recording/SentryCrashReport.c

Lines 816 to 850 in e65226c

	writeBacktrace(const SentryCrashReportWriter *const writer, const char *const key,
	SentryCrashStackCursor *stackCursor)
	{
	writer->beginObject(writer, key);
	{
	writer->beginArray(writer, SentryCrashField_Contents);
	{
	while (stackCursor->advanceCursor(stackCursor)) {
	writer->beginObject(writer, NULL);
	{
	if (stackCursor->symbolicate(stackCursor)) {
	if (stackCursor->stackEntry.imageName != NULL) {
	writer->addStringElement(writer, SentryCrashField_ObjectName,
	sentrycrashfu_lastPathEntry(stackCursor->stackEntry.imageName));
	}
	writer->addUIntegerElement(writer, SentryCrashField_ObjectAddr,
	stackCursor->stackEntry.imageAddress);
	if (stackCursor->stackEntry.symbolName != NULL) {
	writer->addStringElement(writer, SentryCrashField_SymbolName,
	stackCursor->stackEntry.symbolName);
	}
	writer->addUIntegerElement(writer, SentryCrashField_SymbolAddr,
	stackCursor->stackEntry.symbolAddress);
	}
	writer->addUIntegerElement(
	writer, SentryCrashField_InstructionAddr, stackCursor->stackEntry.address);
	}
	writer->endContainer(writer);
	}
	}
	writer->endContainer(writer);
	writer->addIntegerElement(writer, SentryCrashField_Skipped, 0);
	}
	writer->endContainer(writer);
	}

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]

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]

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 🥀