Skip to content

fix: Copy mutable collections before passing to scope observers#7807

Open
antonis wants to merge 2 commits intomainfrom
fix/scope-observer-race-condition
Open

fix: Copy mutable collections before passing to scope observers#7807
antonis wants to merge 2 commits intomainfrom
fix/scope-observer-race-condition

Conversation

@antonis
Copy link
Copy Markdown
Contributor

@antonis antonis commented Apr 16, 2026

📜 Description

Copy mutable collections (_contextDictionary, _tagDictionary, _extraDictionary, _fingerprintArray, _attributesDictionary) before passing them to scope observers in SentryScope.m. This prevents a race condition where observers dispatch async work that iterates the collection after the @synchronized lock is released, while another thread mutates it concurrently.

Also updates the misleading thread-safety comment in SentryWatchdogTerminationScopeObserver.swift to accurately describe the guarantee.

💡 Motivation and Context

Fixes a launch crash (EXC_BAD_ACCESS in objc_msgSend) reported in getsentry/sentry-react-native#5995 affecting <1% of cold launches on iOS 26+. The crash occurs because SentryWatchdogTerminationScopeObserver captures mutable collection references and dispatches async disk writes, racing with concurrent scope mutations.

The public getter methods (context, extras, tags, fingerprints, attributes) already return .copy — this fix makes observer notifications consistent with that pattern.

💚 How did you test it?

  • Added testModifyingFromMultipleThreads_withObserverAsyncDispatch — a concurrent stress test that rapidly mutates scope context/tags/extras/fingerprint from multiple threads while an observer with async dispatch is active
  • All existing SentryScopeSwiftTests (59 tests) pass
  • All existing SentryWatchdogTerminationScopeObserverTests (18 tests) pass
  • make format, make analyze, make build-ios all clean

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

SentryScope passes mutable collections to scope observers inside
@synchronized blocks. Observers like WatchdogTermination dispatch
async work that iterates the collection after the lock is released,
racing with concurrent mutations — causing EXC_BAD_ACCESS.

Copy collections before passing to observers, consistent with
the existing public getter methods that already return copies.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 16, 2026

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).


Bug Fixes 🐛

  • (replay) Keep replayType as buffer for Session Replay triggered by an error by romtsn in #7804
  • Copy mutable collections before passing to scope observers by antonis in #7807
  • Detect development builds via provisioning profile entitlement by denrase in #7702

Internal Changes 🔧

Deps

  • Bump actions/upload-pages-artifact from 4.0.0 to 5.0.0 by dependabot in #7789
  • Bump actions/github-script from 8.0.0 to 9.0.0 by dependabot in #7793

🤖 This preview updates automatically when you update the PR.

@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Apr 16, 2026

@cursor review

@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Apr 16, 2026

@sentry review

Copy link
Copy Markdown

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit dd9ad6b. Configure here.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.130%. Comparing base (1723b14) to head (dd9ad6b).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
Sources/Sentry/SentryScope.m 95.454% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #7807       +/-   ##
=============================================
- Coverage   85.418%   85.130%   -0.289%     
=============================================
  Files          487       487               
  Lines        29195     29207       +12     
  Branches     12632     12625        -7     
=============================================
- Hits         24938     24864       -74     
- Misses        4207      4292       +85     
- Partials        50        51        +1     
Files with missing lines Coverage Δ
...tions/SentryWatchdogTerminationScopeObserver.swift 75.000% <ø> (ø)
Sources/Sentry/SentryScope.m 96.987% <95.454%> (+0.726%) ⬆️

... and 15 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 1723b14...dd9ad6b. Read the comment docs.

@antonis antonis marked this pull request as ready for review April 16, 2026 09:08
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.

Launch crash in SentryWatchdogTerminationAttributesProcessor after upgrading to 8.5.0 with native init

1 participant