ref: Use SentryMutex in SentryBinaryImageCache#8226
Conversation
philprime
requested review from
NinjaLikesCheez,
itaybre,
noahsmartin and
philipphofmann
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3a9d1f4. Configure here.
| return | ||
| } | ||
| let image = imagePtr.pointee | ||
| SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageRemoved(image.address) |
There was a problem hiding this comment.
In-flight start re-registers callbacks
Medium Severity
start() initializes state under SentryMutex, then registers C callbacks outside the lock. If stop() runs in that gap, it clears cache and unregisters callbacks, but the original start() still calls sentrycrashbic_registerAddedCallback / registerRemovedCallback afterward. That leaves global callbacks active while cache stays nil, so replay and later adds are dropped and symbolication can stay broken until another successful start().
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3a9d1f4. Configure here.
📲 Install BuildsiOS
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9787f05 | 1233.67 ms | 1269.00 ms | 35.33 ms |
| 119dc37 | 1217.64 ms | 1250.44 ms | 32.80 ms |
| 31a83fd | 1218.63 ms | 1247.18 ms | 28.55 ms |
| 5cc1f4e | 1223.49 ms | 1258.69 ms | 35.20 ms |
| 92bcc8f | 1233.43 ms | 1270.20 ms | 36.77 ms |
| 943b250 | 1214.69 ms | 1257.32 ms | 42.63 ms |
| 78aad96 | 1232.02 ms | 1262.42 ms | 30.40 ms |
| 1caa212 | 1229.06 ms | 1251.23 ms | 22.17 ms |
| 2446b3d | 1218.36 ms | 1253.98 ms | 35.62 ms |
| 6844d69 | 1228.25 ms | 1265.20 ms | 36.95 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9787f05 | 24.14 KiB | 1.18 MiB | 1.15 MiB |
| 119dc37 | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 31a83fd | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| 5cc1f4e | 24.14 KiB | 1.18 MiB | 1.16 MiB |
| 92bcc8f | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 943b250 | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| 78aad96 | 24.14 KiB | 1.18 MiB | 1.16 MiB |
| 1caa212 | 24.14 KiB | 1.18 MiB | 1.16 MiB |
| 2446b3d | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| 6844d69 | 24.14 KiB | 1.20 MiB | 1.17 MiB |
1 participant
Follow-up from #8214 to remove
NSRecursiveLockfrom the SentryBinaryImageCache for preparing for eventual migration toSynchronization.Mutex(when deployment targets support it). This should also reduce the chance of accidentally accessing state without acquiring a lock first.Extracted this into its own pull request as I know this is a critical section, so please take a closer look during review.
#skip-changelog