Skip to content

Conversation

@supervacuus
Copy link
Collaborator

@supervacuus supervacuus commented Nov 10, 2025

This is a more elaborate, long-term fix to getsentry/sentry-java#4830 than #1444.

It also finishes the work done here: #1088
And fixes the issues raised here: #1353
and here: #906

So, while the driver for this PR is a downstream issue that exposes the signal-unsafety of some parts of the current inproc implementation, it also addresses a much broader range of concerns that regularly affect inproc users on all platforms.

At a high level, it introduces a separate handler thread for inproc, which the signal handler (or UEF on Windows) wakes after it exchanges crash context data.

The idea is that we minimize signal handler/UEF to do the least amount of syscall stuff (or at least the subset documented in the signal-safety man-page), while the handler thread can execute functions outside that range (with limitations, since thread sync and heap allocations are still problematic). This allows us to reuse stdio functionality like formatters without running squarely into UB territory or having to rewrite all utilities to async-signal-safe versions, as in #1444.

There are a few considerable changes to mention:

  • since we run the event construction in a separate handler thread, the use of backtrace() or any unwinder that runs from the "current" instruction address is entirely useless (ignoring the fact that backtrace() was always signal-unsafe to begin with, which itself was the source of crashes, hangs or just empty stack traces).
  • this means we require a "user context"-based stack walker in inproc, which we already partially acknowledged in Using libunwind for mac, since backtrace do not expect thread context… #1088 and fix: support musl on Linux #1233.
  • on Linux, this PR requires libunwind (the nognu implementation, not the llvm one, which is a pure C++ exception unwinder), which is a breaking change (at least in the sense that users now require an additional dependency at build and runtime). This means that the "general" Linux usage is now the same as with the musl libc environments.
  • on macOS, we provide a user context stack-walker based on frame pointer records for arm64 and x86-64, and use the system-provided libunwind for the default stack-trace from a call-site. It turned out that the system-provided libunwind wasn't safe enough to use in the context of the signal handler (either led to hangs or had issues with escaping the trampoline). This means users can now use inproc on macOS again (if their code is compiled without omitting frame pointers, which is always the case by default on macOS).

Further improvements/fixes (summarizing the 30 commits, which I didn't want to squash):

  • the libunwind-based unwinder modules now also validate retrieved ucontext pointers against memory mapping (for Linux and macOS)
  • got rid of all remaining __sync functions and replaced them with __atomic (especially the signal handler blocking logic and the spinlock)
  • rectified the inconsistent usage of C++ new with std::nothrow throughout the affected backend code (including the initialization of crashpad_state_t, which still used malloc and memset although it has std::atomic members)
  • cleaned up the CMake configure phase of the integration test suite.
  • ensures that test fixtures do not end up in macOS bundles
  • fixes build issues with by-default PIE and LTO builds
  • musl is no longer a special case "Linux" in the build script
  • fixes a couple of warnings and test-case instabilities
  • introduce macos-26 build config

TODOs:

  • finish the x86-64 stackwalker for macOS (and clean up the code)
  • Figure out if we need the libbacktrace fallback at all and how to handle it.
  • provide a module-level description of the new mechanism in inproc
  • Decide on having the change
  • Update documentation
    • Advanced usage might be outdated wrt to signal handling of inproc
    • Remove mentions of inproc not working on macOS
    • Clarify the new libunwind dependency on Linux

* use `std::nothrow` `new` consistently to keep exception-free semantics for allocation
* rename static crashpad_handler to have no module-public prefix
* use `nullptr` for arguments where we previously used 0 to clarify that those are pointers
* eliminate the `memset()` of the `crashpad_state_t` initialization since it now contains non-trivially constructable fields (`std::atomic`) and replace it with `new` and an empty value initializer.
…ld, since libraries like libunwind.a might be packaged without PIC.
…ms with architecture prefixes (32-bit Linux)
…stack

also ensure to get the first frame
harmonize libunwind usage
@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 73.17073% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.13%. Comparing base (67a9f8e) to head (4b07add).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
- Coverage   84.66%   84.13%   -0.53%     
==========================================
  Files          58       59       +1     
  Lines        7960     8220     +260     
  Branches     1556     1610      +54     
==========================================
+ Hits         6739     6916     +177     
- Misses       1058     1111      +53     
- Partials      163      193      +30     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vaind vaind removed their request for review January 28, 2026 10:20
@supervacuus supervacuus marked this pull request as draft February 9, 2026 16:47
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


**Breaking**:

- inproc(Linux): the `inproc` backend on Linux now depends on "nognu" `libunwind`. ([#1446](https://github.com/getsentry/sentry-native/pull/1446))
Copy link

Choose a reason for hiding this comment

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

  • 🚫 The changelog entry seems to be part of an already released section ## 0.12.6.
    Consider moving the entry to the ## Unreleased section, please.

* introduces state-machine for in-progress crash handling vs other crashing threads
* extract async-safe logging macros
* adds arm64e support PAC registers (access to opaque fp, lr, sp, and pc)
* skip on_crash if we handle a failing handler thread (TODO: also before_send)
* handle failed pipe and semaphore signaling to handler thread
* introduce even more fall backs to handling inside the signal handler where it makes sense (i.e. failed attempt to signal handler thread)
…ntegration test executables with the same parametrization as the core artifacts and "example".
… unstable... it works great locally and should remain for local testing and debugging though.
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.

3 participants