sentry-cocoa icon indicating copy to clipboard operation
sentry-cocoa copied to clipboard

fix: Add date provider to SentryHttpTransport constructor

Open philprime opened this issue 6 months ago • 2 comments

This PR fixes a deadlock in SentryHttpTransport by injecting a date provider instead of accessing it via the SentryDependencyContainer.sharedInstance.

Tests were experiencing deadlocks due to a circular dependency between SentryDependencyContainer and SentryReachability:

Deadlock Scenario:

  • Thread 1 (main): Holds sentryDependencyContainerInstanceLock during reset() → trying to acquire sentry_reachability_observers lock in removeAllObservers
  • Thread 112/132 (reachability callbacks): Hold sentry_reachability_observers lock → trying to acquire sentryDependencyContainerInstanceLock via SentryDependencyContainer.sharedInstance.dateProvider
Thread 1 Queue : com.apple.main-thread (serial)
#0	0x00000001010e1ae0 in __ulock_wait2 ()
#1	0x0000000100c6539c in _os_unfair_lock_lock_slow ()
#2	0x00000001800898ec in objc_sync_enter ()
#3	0x0000000120f3d988 in -[SentryReachability removeAllObservers] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryReachability.m:233
#4	0x0000000120f956e0 in +[SentryDependencyContainer reset] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryDependencyContainer.m:135
#5	0x0000000120f75484 in +[SentrySDK close] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentrySDK.m:639
#6	0x0000000123285dc8 in static TestCleanup.clearTestState() at /Volumes/Developer/getsentry/sentry-cocoa/SentryTestUtils/ClearTestState.swift:25
#7	0x0000000123285c98 in clearTestState() at /Volumes/Developer/getsentry/sentry-cocoa/SentryTestUtils/ClearTestState.swift:5
#8	0x0000000122c6c630 in SentryHubTests.tearDown() at /Volumes/Developer/getsentry/sentry-cocoa/Tests/SentryTests/SentryHubTests.swift:88
#9	0x0000000122c6c66c in @objc SentryHubTests.tearDown() ()
#10	0x00000001012fdb18 in __86-[XCTestCase _performTearDownSequenceWithSelector:permittingControlFlowInterruptions:]_block_invoke_2 ()
#11	0x00000001012fddb4 in __79-[XCTestCase _performFailableTearDownBlock:permittingControlFlowInterruptions:]_block_invoke ()
#12	0x00000001012e91fc in -[XCTestCase(XCTIssueHandling) _caughtUnhandledDeveloperExceptionPermittingControlFlowInterruptions:caughtInterruptionException:whileExecutingBlock:] ()
#13	0x00000001012fdba0 in -[XCTestCase _performFailableTearDownBlock:permittingControlFlowInterruptions:] ()
#14	0x00000001012fd7dc in __86-[XCTestCase _performTearDownSequenceWithSelector:permittingControlFlowInterruptions:]_block_invoke ()
Thread 112 Queue : io.sentry.cocoa.connectivity (serial)
#0	0x00000001010e1ae0 in __ulock_wait2 ()
#1	0x0000000100c6539c in _os_unfair_lock_lock_slow ()
#2	0x00000001800898ec in objc_sync_enter ()
#3	0x0000000120f95530 in +[SentryDependencyContainer sharedInstance] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryDependencyContainer.m:115
#4	0x0000000120f3aa00 in -[SentryHttpTransport sendAllCachedEnvelopes] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryHttpTransport.m:339
#5	0x0000000120f38f30 in -[SentryHttpTransport connectivityChanged:typeDescription:] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryHttpTransport.m:110
#6	0x0000000120f3cf98 in SentryConnectivityCallback at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryReachability.m:121
#7	0x0000000120f3d1c8 in SentryConnectivityActualCallback at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryReachability.m:145
#8	0x00000001807db684 in reachPerformAndUnlock ()
#9	0x00000001807dbfec in ____SCNetworkReachabilityRestartResolver_block_invoke_2 ()
#10	0x0000000187e734e8 in invocation function for block in nw_resolver_update_client(NWConcrete_nw_resolver*, void () block_pointer) ()
#11	0x000000018017c788 in _dispatch_call_block_and_release ()
#12	0x0000000180197278 in _dispatch_client_callout ()
#13	0x0000000180185ad0 in _dispatch_lane_serial_drain ()
#14	0x0000000180186590 in _dispatch_lane_invoke ()
#15	0x0000000180191380 in _dispatch_root_queue_drain_deferred_wlh ()
#16	0x00000001801909f4 in _dispatch_workloop_worker_thread ()
#17	0x0000000100c06bcc in _pthread_wqthread ()
Thread 132 Queue : io.sentry.default (serial)
#0	0x00000001010e1ae0 in __ulock_wait2 ()
#1	0x0000000100c6539c in _os_unfair_lock_lock_slow ()
#2	0x00000001800898ec in objc_sync_enter ()
#3	0x0000000120f95530 in +[SentryDependencyContainer sharedInstance] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryDependencyContainer.m:115
#4	0x0000000120fbef2c in -[SentryFileManager deleteOldEnvelopesFromPath:] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryFileManager.m:964
#5	0x0000000120fbee1c in -[SentryFileManager deleteOldEnvelopesFromAllSentryPaths] at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryFileManager.m:957
#6	0x0000000120fba350 in __43-[SentryFileManager deleteOldEnvelopeItems]_block_invoke at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryFileManager.m:292
#7	0x0000000120eb1198 in __53-[SentryDispatchQueueWrapper dispatchAsyncWithBlock:]_block_invoke at /Volumes/Developer/getsentry/sentry-cocoa/Sources/Sentry/SentryDispatchQueueWrapper.m:31
#8	0x000000018017c788 in _dispatch_call_block_and_release ()
#9	0x0000000180197278 in _dispatch_client_callout ()
#10	0x0000000180185ad0 in _dispatch_lane_serial_drain ()
#11	0x0000000180186590 in _dispatch_lane_invoke ()
#12	0x0000000180191380 in _dispatch_root_queue_drain_deferred_wlh ()
#13	0x00000001801909f4 in _dispatch_workloop_worker_thread ()
#14	0x0000000100c06bcc in _pthread_wqthread ()

Solution

Implemented dependency injection to eliminate SentryHttpTransport's dependency on the global SentryDependencyContainer.sharedInstance, breaking the circular dependency.

Testing

  • ✅ All existing tests pass

#skip-changelog

philprime avatar Jun 20 '25 13:06 philprime

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.150%. Comparing base (5652830) to head (1c5cf1f). Report is 9 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5439       +/-   ##
=============================================
- Coverage   86.179%   86.150%   -0.030%     
=============================================
  Files          399       399               
  Lines        34616     34621        +5     
  Branches     14972     14980        +8     
=============================================
- Hits         29832     29826        -6     
- Misses        4739      4749       +10     
- Partials        45        46        +1     
Files with missing lines Coverage Δ
Sources/Sentry/SentryClient.m 98.465% <100.000%> (+0.002%) :arrow_up:
Sources/Sentry/SentryHttpTransport.m 98.168% <100.000%> (+0.006%) :arrow_up:
Sources/Sentry/SentryTransportFactory.m 100.000% <100.000%> (ø)

... and 9 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 5652830...1c5cf1f. Read the comment docs.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jun 20 '25 14:06 codecov[bot]

Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1239.24 ms 1254.77 ms 15.53 ms
Size 23.75 KiB 847.44 KiB 823.69 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
077e94086236b0be69cb0982b64053b94f2c9ea4 1227.31 ms 1246.51 ms 19.20 ms
15b8c6111854bacf68624e765d99adcf93bfc987 1223.16 ms 1244.83 ms 21.67 ms
bc0c36e91c733f434511cb832b636fdc8ad90330 1220.41 ms 1239.45 ms 19.04 ms
455619dd977fcfe15f8d8294a8d217fcb0b90d15 1231.40 ms 1237.70 ms 6.30 ms
f8833c443d047f0c8dac5710e52b4c0624fc52ce 1236.45 ms 1252.82 ms 16.37 ms
728804f93e9f74d591108dd8a8d2df61a645c02b 1235.26 ms 1254.20 ms 18.94 ms
6e342acf0107a6421fa9221f9ecd93c5f3ef0b3b 1220.24 ms 1240.14 ms 19.90 ms
46f5eb8f90236e0b538f5ff1c237e290c12a25e6 1200.09 ms 1231.38 ms 31.29 ms
3912b16e2a203c2dd8155aa48aaf4c4dc10199e3 1230.54 ms 1249.65 ms 19.11 ms
061c982c8616fa6ca7f0762998f39a8f4d8d2fc2 1226.33 ms 1243.65 ms 17.33 ms

App size

Revision Plain With Sentry Diff
077e94086236b0be69cb0982b64053b94f2c9ea4 21.58 KiB 706.97 KiB 685.39 KiB
15b8c6111854bacf68624e765d99adcf93bfc987 20.76 KiB 419.67 KiB 398.91 KiB
bc0c36e91c733f434511cb832b636fdc8ad90330 22.31 KiB 775.27 KiB 752.95 KiB
455619dd977fcfe15f8d8294a8d217fcb0b90d15 20.76 KiB 432.87 KiB 412.11 KiB
f8833c443d047f0c8dac5710e52b4c0624fc52ce 21.58 KiB 422.66 KiB 401.08 KiB
728804f93e9f74d591108dd8a8d2df61a645c02b 22.85 KiB 411.76 KiB 388.91 KiB
6e342acf0107a6421fa9221f9ecd93c5f3ef0b3b 20.76 KiB 436.66 KiB 415.90 KiB
46f5eb8f90236e0b538f5ff1c237e290c12a25e6 20.76 KiB 432.37 KiB 411.61 KiB
3912b16e2a203c2dd8155aa48aaf4c4dc10199e3 21.58 KiB 625.82 KiB 604.24 KiB
061c982c8616fa6ca7f0762998f39a8f4d8d2fc2 21.58 KiB 683.51 KiB 661.93 KiB

github-actions[bot] avatar Jun 20 '25 14:06 github-actions[bot]