fix(macos): Fix thread safety in NSException capture by philprime · Pull Request #7672 · getsentry/sentry-cocoa

philprime · 2026-03-11T10:48:10Z

Description

Adds thread-safe atomic operations to the SentryNSExceptionCaptureHelper class introduced in #7510, fixing a race condition in macOS exception handling.

Problem

PR #7510 recently refactored macOS exception handling and introduced SentryNSExceptionCaptureHelper with a deduplication flag to prevent duplicate captures when _crashOnException: is called from within reportException:.

However, the flag uses a simple static BOOL, which is not thread-safe. When multiple threads throw exceptions simultaneously, race conditions can occur:

Thread A sets flag = YES, starts capturing exception
Thread B also sets flag = YES, starts capturing same exception
Both threads can bypass the deduplication check
Duplicate reports are sent

Solution

Replace static BOOL with C11 atomic operations:

static BOOL _insideReportException → static _Atomic BOOL _insideReportException
Direct assignment → atomic_store(&_insideReportException, YES/NO)
Direct read → atomic_load(&_insideReportException)

This ensures thread-safe read-modify-write operations on the deduplication flag.

Changes

Modified: SentryNSExceptionCaptureHelper.m
- Add #import <stdatomic.h>
- Change flag to _Atomic BOOL
- Use atomic_store() and atomic_load() for all accesses
- Add comment explaining thread safety

Testing

✅ All existing exception handling tests passing
✅ Tests verify atomic flag prevents duplicates during nested calls
✅ macOS build and test suite passes

Replace static BOOL flag with C11 atomic to prevent race conditions when multiple threads throw exceptions simultaneously. The previous implementation could allow duplicate exception captures when _crashOnException: was called from within reportException:. Key changes: - Add SentryNSExceptionCaptureHelper with atomic flag - Swizzle both reportException: and _crashOnException: methods - Remove SentryUseNSExceptionCallstackWrapper (no longer needed) - Simplify exception handling flow The atomic flag ensures thread-safe deduplication: when reportException: captures an exception and calls [super reportException:], which internally dispatches to _crashOnException:, the second call is skipped to avoid duplicate reports.

github-actions · 2026-03-11T10:49:04Z

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).

New Features ✨

(feedback) Implement shake gesture detection by antonis in #7579
(SPM) Add package traits for UI framework opt-out and macOS CLI sample by philprime in #7578
Use full flamegraph for metrickit app hangs by noahsmartin in #7185

Bug Fixes 🐛

(macos) Fix thread safety in NSException capture by philprime in #7672

Capture instance and class method [NSApplication \_crashOnException] exceptions by denrase in #7510
Capture transactions during launch profiling window by philipphofmann in #7602

Internal Changes 🔧

Deps

Bump fastlane-plugin-sentry from 2.1.1 to 2.2.0 by dependabot in #7626
Bump mikepenz/action-junit-report from 6.3.0 to 6.3.1 by dependabot in #7630
Bump actions/setup-node from 6.2.0 to 6.3.0 by dependabot in #7627
Bump getsentry/craft/.github/workflows/changelog-preview.yml from 2.23.0 to 2.23.2 by dependabot in #7628
Bump getsentry/craft from 2.23.0 to 2.23.2 by dependabot in #7629
Bump ruby/setup-ruby from 1.288.0 to 1.290.0 by dependabot in #7631

Samples

Restructure iOS-SwiftUI-Widgets sample by philprime in #7658
Restructure iOS15-SwiftUI sample by philprime in #7657
Restructure watchOS-Swift sample by philprime in #7614
Restructure visionOS-Swift sample by philprime in #7616
Restructure visionOS-SwiftUI-SPM sample by philprime in #7615

Other

(ci) Adds a Nightly Job with unit tests for less frequent devices by itaybre in #7632
(test-samples) Restructure SwiftUITestSample and SwiftUICrashTest by philprime in #7612
Fix watchOS tests and add them to nightly job by itaybre in #7633
Add crash when SentryInitializeForGettingSubclassesNotCalled is… by noahsmartin in #7637
Fix flaky HangTracker deallocated test by noahsmartin in #7639

_{🤖 This preview updates automatically when you update the PR.}

Sources/Sentry/SentryCrashExceptionApplication.m

…h-on-exception # Conflicts: # Sources/Sentry/SentryNSExceptionCaptureHelper.m

sentry · 2026-03-11T11:09:54Z

Sentry Build Distribution

App	Version	Configuration
App	9.6.0 (1)	Release

codecov · 2026-03-11T11:21:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.324%. Comparing base (2c7a266) to head (adfb2ba).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #7672       +/-   ##
=============================================
+ Coverage   85.307%   85.324%   +0.016%     
=============================================
  Files          485       485               
  Lines        28838     28837        -1     
  Branches     12523     12521        -2     
=============================================
+ Hits         24601     24605        +4     
+ Misses        4190      4185        -5     
  Partials        47        47

see 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c7a266...adfb2ba. Read the comment docs.

Revert atomic operations changes to SentryNSExceptionCaptureHelper.m that were extracted to a separate PR #7672 for independent review. This keeps our PR focused on the SentryObjC wrapper SDK functionality. Refs #6342 Refs #7672

github-actions · 2026-03-11T11:28:18Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1235.98 ms	1258.72 ms	22.74 ms
Size	24.14 KiB	1.12 MiB	1.10 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`b0c71c8`	1206.29 ms	1242.13 ms	35.84 ms
`e701dc8`	1215.89 ms	1254.06 ms	38.17 ms
`88592c1`	1221.80 ms	1259.98 ms	38.18 ms
`1b74fd4`	1219.43 ms	1250.17 ms	30.73 ms
`6108e4c`	1212.31 ms	1251.80 ms	39.49 ms
`56c05ae`	1218.02 ms	1253.30 ms	35.28 ms
`19f3e6b`	1231.17 ms	1257.02 ms	25.85 ms
`2d1826b`	1216.96 ms	1254.05 ms	37.09 ms
`d68691e`	1221.48 ms	1248.13 ms	26.65 ms
`c424b6a`	1220.38 ms	1248.18 ms	27.80 ms

App size

Revision	Plain	With Sentry	Diff
`b0c71c8`	24.14 KiB	1.08 MiB	1.06 MiB
`e701dc8`	24.14 KiB	1.06 MiB	1.04 MiB
`88592c1`	24.14 KiB	1.12 MiB	1.10 MiB
`1b74fd4`	24.14 KiB	1.10 MiB	1.08 MiB
`6108e4c`	24.14 KiB	1.11 MiB	1.09 MiB
`56c05ae`	24.14 KiB	1.10 MiB	1.08 MiB
`19f3e6b`	24.14 KiB	1.10 MiB	1.08 MiB
`2d1826b`	24.14 KiB	1.12 MiB	1.09 MiB
`d68691e`	24.14 KiB	1.12 MiB	1.09 MiB
`c424b6a`	24.14 KiB	1.06 MiB	1.04 MiB

denrase

The initial issue here was that _crashOnException can now also be called individually, and not internally by reportException. So we additionally swizzle it.

But then in the case of reportException calling _crashOnException, we need to guard against it, that's why we have the flag.

As for concurrency, I can't find any documentation pointing to if this needs to be thread-safe, so I can't say if these NSApplication methods are only being called form the main thread.

If we'd want to do this, we'd need to make sure the individual threads have the interleaving protection, which i don't think we have with the atomic? Quick search turned up _Thread_local , but I am not familiar with it and we have not used it in the codebase.

static _Thread_local BOOL _insideReportException = NO;

philprime requested review from itaybre, noahsmartin and philipphofmann as code owners March 11, 2026 10:48

philprime marked this pull request as draft March 11, 2026 10:48

docs: Add changelog entry for NSException thread safety fix

3e4252e

sentry bot reviewed Mar 11, 2026

View reviewed changes

Sources/Sentry/SentryCrashExceptionApplication.m Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into philprime/atomic-cras…

adfb2ba

…h-on-exception # Conflicts: # Sources/Sentry/SentryNSExceptionCaptureHelper.m

philprime marked this pull request as ready for review March 11, 2026 11:06

philprime added the ready-to-merge Use this label to trigger all PR workflows label Mar 11, 2026

philprime requested a review from denrase March 11, 2026 11:06

philprime mentioned this pull request Mar 11, 2026

feat(objc): Add SentryObjC wrapper SDK #7598

Draft

7 tasks

philprime enabled auto-merge (squash) March 11, 2026 11:26

denrase reviewed Mar 11, 2026

View reviewed changes

philprime disabled auto-merge March 11, 2026 12:50

philprime assigned philprime and unassigned philprime Mar 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(macos): Fix thread safety in NSException capture#7672

fix(macos): Fix thread safety in NSException capture#7672
philprime wants to merge 3 commits intomainfrom
philprime/atomic-crash-on-exception

philprime commented Mar 11, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 11, 2026 •

edited

Loading

New Features ✨

Bug Fixes 🐛

Internal Changes 🔧

Deps

Samples

Other

Uh oh!

Uh oh!

sentry bot commented Mar 11, 2026

Uh oh!

codecov bot commented Mar 11, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 11, 2026

Baseline results on branch: main

Startup times

App size

Uh oh!

denrase left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

philprime commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Changes

Testing

Related

Uh oh!

github-actions bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Semver Impact of This PR

New Features ✨

Bug Fixes 🐛

Internal Changes 🔧

Deps

Samples

Other

Uh oh!

Uh oh!

sentry bot commented Mar 11, 2026

Sentry Build Distribution

Uh oh!

codecov bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 11, 2026

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Uh oh!

denrase left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

philprime commented Mar 11, 2026 •

edited

Loading

github-actions bot commented Mar 11, 2026 •

edited

Loading

codecov bot commented Mar 11, 2026 •

edited

Loading