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

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
85 4 81 1
View the top 3 failed test(s) by shortest run time
SentryCrashBinaryImageCacheTests::testRemoveImageAddAgain
Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m:309 - ((counter) equal to (expected)) failed: ("0") is not equal to ("5")
SentryCrashBinaryImageCacheTests::testRemoveImageFromBeginning
Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m:309 - ((counter) equal to (expected)) failed: ("0") is not equal to ("5")
SentryCrashBinaryImageCacheTests::testRemoveImageFromTail
Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m:309 - ((counter) equal to (expected)) failed: ("0") is not equal to ("5")
SentryTests.PrivateSentrySDKOnlyTests::testProfilingStartAndCollect
Stack Traces | 0s run time
.../Tests/SentryTests/PrivateSentrySDKOnlyTests.swift:271 - XCTAssertGreaterThan failed: ("0") is not greater than ("0")
SentryTests.SentryFileManagerTests::testCreateDirectoryIfNotExists_successful_shouldNotLogError
Stack Traces | 0s run time
.../SentryTests/Helper/SentryFileManagerTests.swift:1056 - XCTAssertEqual failed: ("1") is not equal to ("0")

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

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1206.16 ms 1222.51 ms 16.35 ms
Size 24.14 KiB 1.08 MiB 1.06 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b5c1b24 1217.94 ms 1248.76 ms 30.82 ms
83bf9af 1213.30 ms 1234.18 ms 20.89 ms
c424b6a 1220.38 ms 1248.18 ms 27.80 ms
b206854 1228.68 ms 1250.76 ms 22.08 ms
7e93d85 1205.28 ms 1243.71 ms 38.44 ms
3b01aaf 1194.98 ms 1210.36 ms 15.38 ms
79e2bb8 1216.37 ms 1242.42 ms 26.05 ms
50e7b3e 1221.54 ms 1250.81 ms 29.27 ms
a7d7fb6 1209.53 ms 1235.91 ms 26.38 ms
2e5230b 1207.41 ms 1240.41 ms 33.00 ms

App size

Revision Plain With Sentry Diff
b5c1b24 24.14 KiB 1.06 MiB 1.04 MiB
83bf9af 24.14 KiB 1.04 MiB 1.02 MiB
c424b6a 24.14 KiB 1.06 MiB 1.04 MiB
b206854 24.14 KiB 1.07 MiB 1.04 MiB
7e93d85 24.14 KiB 1.06 MiB 1.04 MiB
3b01aaf 24.14 KiB 1.06 MiB 1.04 MiB
79e2bb8 24.14 KiB 1.04 MiB 1.02 MiB
50e7b3e 24.14 KiB 1.04 MiB 1.02 MiB
a7d7fb6 24.14 KiB 1.06 MiB 1.04 MiB
2e5230b 24.14 KiB 1.04 MiB 1.02 MiB

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.

4 participants