Skip to content

Conversation

@noahsmartin
Copy link
Contributor

This is a POC for how I think we should solve #4618

It fixes an existing race condition and moves the image handler to a background thread. Synchronization is now done safely with atomics so the crash handler can read from the added images without risk of race conditions (which existed before).

#skip-changelog

@github-actions
Copy link
Contributor

github-actions bot commented Jan 23, 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).


This PR will not appear in the changelog.


🤖 This preview updates automatically when you update the PR.

@codecov
Copy link

codecov bot commented Jan 23, 2026

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4466 1 4465 51
View the top 2 failed test(s) by shortest run time
SentryCrashBinaryImageCacheTests::testSentryBinaryImageCacheIntegration
Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m:295 - ((5) equal to (imageCache.cache.count)) failed: ("5") is not equal to ("6") - Cache should start with 5 images but contained (
SentryCrashBinaryImageCacheTests::testStopCache
Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m:318 - ((counter) equal to (expected)) failed: ("6") is not equal to ("5") - Cache should have 5 images but contained (

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@noahsmartin noahsmartin added the ready-to-merge Use this label to trigger all PR workflows label Jan 23, 2026
add_dyld_image(mh);
}

void dyld_tracker_start(void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should have sentry's prefix

@philipphofmann
Copy link
Member

@noahsmartin, I didn't check the code yet, as this is still a draft. Please be aware that they also need the loaded binary image in the UIViewControllerSwizzling, as pointed out here #4618 (comment). If the proposal respects this, then excellent.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2026

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1209.61 ms 1251.52 ms 41.91 ms
Size 24.14 KiB 1.10 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
015d376 1187.58 ms 1211.28 ms 23.70 ms
680923d 1211.40 ms 1241.00 ms 29.60 ms
edcda5a 1217.52 ms 1248.72 ms 31.20 ms
0d145c3 1224.04 ms 1255.34 ms 31.30 ms
37ebda0 1201.62 ms 1223.76 ms 22.13 ms
d427374 1226.81 ms 1257.74 ms 30.94 ms
29b6704 1216.31 ms 1251.91 ms 35.60 ms
d4dd2bd 1218.59 ms 1255.21 ms 36.61 ms
a7d7fb6 1209.53 ms 1235.91 ms 26.38 ms
fd24b7c 1222.39 ms 1259.67 ms 37.28 ms

App size

Revision Plain With Sentry Diff
015d376 24.14 KiB 1.09 MiB 1.07 MiB
680923d 24.14 KiB 1.10 MiB 1.08 MiB
edcda5a 24.14 KiB 1.08 MiB 1.06 MiB
0d145c3 24.14 KiB 1.07 MiB 1.05 MiB
37ebda0 24.14 KiB 1.10 MiB 1.07 MiB
d427374 24.14 KiB 1.09 MiB 1.07 MiB
29b6704 24.14 KiB 1.06 MiB 1.04 MiB
d4dd2bd 24.14 KiB 1.10 MiB 1.08 MiB
a7d7fb6 24.14 KiB 1.06 MiB 1.04 MiB
fd24b7c 24.14 KiB 1.07 MiB 1.04 MiB

Previous results on branch: imageCacheAsync

Startup times

Revision Plain With Sentry Diff
629dd8e 1225.23 ms 1252.12 ms 26.89 ms
f02c17c 1212.57 ms 1246.06 ms 33.49 ms
823a3af 1206.16 ms 1222.51 ms 16.35 ms
272a47a 1233.83 ms 1249.33 ms 15.50 ms
496a3e1 1221.53 ms 1251.12 ms 29.59 ms
fb2a4b2 1217.60 ms 1244.74 ms 27.15 ms
7847014 1222.58 ms 1241.55 ms 18.97 ms
45a6e74 1217.39 ms 1254.95 ms 37.56 ms

App size

Revision Plain With Sentry Diff
629dd8e 24.14 KiB 1.10 MiB 1.08 MiB
f02c17c 24.14 KiB 1.10 MiB 1.07 MiB
823a3af 24.14 KiB 1.08 MiB 1.06 MiB
272a47a 24.14 KiB 1.10 MiB 1.08 MiB
496a3e1 24.14 KiB 1.10 MiB 1.08 MiB
fb2a4b2 24.14 KiB 1.10 MiB 1.08 MiB
7847014 24.14 KiB 1.10 MiB 1.08 MiB
45a6e74 24.14 KiB 1.10 MiB 1.08 MiB

@noahsmartin noahsmartin force-pushed the imageCacheAsync branch 6 times, most recently from 52e7b6d to 51622cf Compare February 9, 2026 17:01
@noahsmartin noahsmartin force-pushed the imageCacheAsync branch 2 times, most recently from f9ef564 to 2ca37c1 Compare February 9, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Use this label to trigger all PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants