Skip to content

feat(native): add thread stackwalk mode option - #2026

Merged
tustanivsky merged 2 commits into
masterfrom
feat/thread-stackwalk-mode
Aug 26, 2026
Merged

feat(native): add thread stackwalk mode option#2026
tustanivsky merged 2 commits into
masterfrom
feat/thread-stackwalk-mode

Conversation

@tustanivsky

@tustanivsky tustanivsky commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

The crash daemon walks and symbolicates every thread of the crashed process. On applications with a high thread count this dominates the time needed to collect a crash.

sentry_options_set_thread_stackwalk_mode allows limiting client-side stackwalking to the crashed thread. Threads that did not crash are still reported with their id and name, only their stacktrace is omitted, so the thread list stays intact. On macOS the mode is also honored in the signal handler, which otherwise writes a stack copy per thread to disk.

Defaults to SENTRY_THREAD_STACKWALK_MODE_ALL, so existing behavior is unchanged.

Testing

Unreal Engine 5.8 editor crash on Windows, native backend with crash_reporting_mode=NATIVE_WITH_MINIDUMP, 134 threads and 1189 modules:

Phase CRASHED_ONLY ALL
Walk non-crashed threads 0 ms (0 threads) 2013 ms (133)
build_native_event total 548 ms 2557 ms
Crash processing, end to end ~2 s ~5 s
Envelope size 2.5 MB 2.7 MB

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 39ea8ae

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.19048% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.35%. Comparing base (dec0e5e) to head (39ea8ae).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2026      +/-   ##
==========================================
+ Coverage   74.24%   74.35%   +0.11%     
==========================================
  Files         104      104              
  Lines       26584    26603      +19     
  Branches     4838     4841       +3     
==========================================
+ Hits        19736    19781      +45     
+ Misses       5499     5472      -27     
- Partials     1349     1350       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tustanivsky
tustanivsky requested review from JoshuaMoelans, jpnurmi and mujacica and removed request for JoshuaMoelans and jpnurmi August 26, 2026 10:45
@jpnurmi

jpnurmi commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

This makes me wonder what the use case is for client-side stackwalking outside the external crash reporter users. Given how expensive it is, what if we actually had a NONE mode and possibly even defaulted to that unless an external crash reporter is used?

I don't have the history of CRASHPAD_ENABLE_STACKTRACE for comparison/reference and especially why it's off by default, but I suspect it might be because of the DbgHelp and libunwind-ptrace dependencies. A NONE option would nicely leave the door open for turning those into optional runtime-loaded dependencies. 🤔

@tustanivsky

Copy link
Copy Markdown
Collaborator Author

what if we actually had a NONE mode and possibly even defaulted to that

Currently, we can switch the crash reporting mode to MINIDUMP to disable client-side stackwalking entirely so adding NONE feels a bit redundant as that would mean we could configure the same behavior via two different options.

what the use case is for client-side stackwalking outside the external crash reporter users

It allows us to still get a symbolicated stack trace for captured crashes on the event page even when debug symbols haven't been uploaded. For instance, this is a common scenario with the Unreal Editor where we deliberately skip the uploads for this target:

image

@jpnurmi

jpnurmi commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

what if we actually had a NONE mode and possibly even defaulted to that

Currently, we can switch the crash reporting mode to MINIDUMP to disable client-side stackwalking entirely so adding NONE feels a bit redundant as that would mean we could configure the same behavior via two different options.

Seems confusing and unintuitive because with Crashpad, client-side stackwalking specifically applies to minidumps. If Native's client-side stackwalking was later implemented for minidumps, would NONE then make sense? 🤯

what the use case is for client-side stackwalking outside the external crash reporter users

It allows us to still get a symbolicated stack trace for captured crashes on the event page even when debug symbols haven't been uploaded. For instance, this is a common scenario with the Unreal Editor where we deliberately skip the uploads for this target:

Ah, good to know, thanks! 🙏

@tustanivsky

Copy link
Copy Markdown
Collaborator Author

If Native's client-side stackwalking was later implemented for minidumps, would NONE then make sense?

Yes, if stackwalking becomes supported in Native's minidump mode (which should make its behavior similar to Crashpad's CRASHPAD_ENABLE_STACKTRACE configuration), having a way to opt out of it there could be useful 🤔

I'm still not sure whether NONE should be the default (if/once it's available) as it seems to defeat the whole purpose of the native crash reporting mode.

@jpnurmi

jpnurmi commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

I'm still not sure whether NONE should be the default (if/once it's available) as it seems to defeat the whole purpose of the native crash reporting mode.

My bad, I went too far with the NONE-as-default proposal. I didn't consider the scenario where the server wouldn't have symbols available. So an opt-out might make sense, but I don't think anymore that we necessarily need to add it to this PR at all. How about we just make the sentry_thread_stackwalk_mode_t values start from 1 so that we reserve the spot for a potential SENTRY_THREAD_STACKWALK_MODE_NONE=0 for later?

The crash daemon walks and symbolicates every thread of the crashed
process. On applications with a high thread count, such as editor or
tooling builds, this dominates the time needed to collect a crash.

Add `sentry_options_set_thread_stackwalk_mode` to limit client-side
stackwalking to the crashed thread. Threads that did not crash are still
reported with their id and name, only their stacktrace is omitted, so the
thread list stays intact. On macOS the mode is also honored in the signal
handler, which otherwise writes a stack copy per thread to disk.

Defaults to SENTRY_THREAD_STACKWALK_MODE_ALL, preserving the previous
behavior.
@tustanivsky
tustanivsky force-pushed the feat/thread-stackwalk-mode branch from ba90ffe to 064339b Compare August 26, 2026 13:35
@tustanivsky
tustanivsky merged commit 3d74596 into master Aug 26, 2026
70 checks passed
@tustanivsky
tustanivsky deleted the feat/thread-stackwalk-mode branch August 26, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants