fix: dispatch UIApplication.applicationState read to main thread on SDK start#8047
fix: dispatch UIApplication.applicationState read to main thread on SDK start#8047tsushanth wants to merge 2 commits into
Conversation
|
Friendly bump — GitHub still shows "Conflicting" but a fresh local merge against current |
SentryThreadsafeApplication.init reads UIApplication.applicationState synchronously from the calling thread. When SentrySDK.start runs on a background thread or a non-main actor (a documented but discouraged pattern), Xcode emits a `-[UIApplication applicationState] must be used from main thread only` runtime warning on every launch. Hop to the main thread for that single property read. The same dispatchSyncOnMainQueue helper is used by other readers in the file (internal_getWindows, internal_getActiveWindowSize) with the same 10 ms timeout — on timeout we fall back to the existing `.active` default and the notification observers correct the state on the next foreground or background transition. The current behavior when start is called on the main thread is unchanged. Closes getsentry#6591
c843354 to
9f8b292
Compare
NinjaLikesCheez
left a comment
There was a problem hiding this comment.
Thank you for your contribution! I have one small nitpick which is to correct a comment - please request a review again when you've addressed it. Thanks!
| // the value safely. The 10ms timeout matches the pattern used elsewhere | ||
| // in this file (e.g. internal_getWindows) — if main is contended we fall |
There was a problem hiding this comment.
nit: please correct this comment to remove 'in this file'.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8047 +/- ##
=============================================
- Coverage 87.406% 87.232% -0.174%
=============================================
Files 559 559
Lines 32167 32183 +16
Branches 13157 13134 -23
=============================================
- Hits 28116 28074 -42
- Misses 4004 4060 +56
- Partials 47 49 +2
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Summary
SentryThreadsafeApplication.initreadsUIApplication.applicationStatesynchronously from the calling thread. WhenSentrySDK.startruns on a background thread or a non-main actor (a documented but discouraged pattern — see #6591), Xcode emits a runtime warning on every launch:The issue author's use case is a non-main-isolated error-reporting service that wraps
SentrySDK.start. A maintainer agreed it's worth fixing.Root cause
ThreadSafeApplication.swift:13–14:unsafeApplicationStateis a direct read ofUIApplication.applicationState, which UIKit gates onThread.isMainThreadand reports via the runtime issue checker.Fix
Hop to the main thread for that single property read using the existing
Dependencies.dispatchQueueWrapper.dispatchSyncOnMainQueue(_:timeout:)helper — the same pattern already used elsewhere in this file (internal_getWindows,internal_getActiveWindowSize,internal_relevantViewControllersNames) with the same 10 ms timeout. On timeout we fall back to the existing.activedefault that the null-application branch uses, and the notification observers registered immediately afterward correct the state on the next foreground/background transition. The behavior whenstartis called on the main thread is unchanged.if let application = applicationProvider() { - _internalState = application.unsafeApplicationState + if Thread.isMainThread { + _internalState = application.unsafeApplicationState + } else { + var stateOnMain: UIApplication.State = .active + Dependencies.dispatchQueueWrapper.dispatchSyncOnMainQueue({ + stateOnMain = application.unsafeApplicationState + }, timeout: 0.01) + _internalState = stateOnMain + } } else { SentrySDKLog.warning("Application is null in SentryThreadsafeApplication") _internalState = .active }Test plan
testInit_OffMainThread_ReadsApplicationStateOnMainThreadtoSentryThreadsafeApplicationTests. The test dispatches init onto a background queue and asserts (via a newunsafeApplicationStateReadOnMainThreadtracker onTestSentryUIApplication) that the underlying property access landed on the main thread, and that the value is correctly propagated to_internalState.xcodebuild buildon theSentryscheme (iOS Simulator, iPhone 17 Pro, iOS 26.4.1) — BUILD SUCCEEDED with the production-code change in place.xcodebuild testcouldn't run the new test in this PR's checkout —SentryTestUtils/Sources/ClearTestState.swift:52callsSentryAppStartMeasurementProvider.reset()which Swift can't resolve (confirmed pre-existing onmainwithout my changes). This appears to be a separate test-infra build issue; deferring to CI for the actual run.Changelog
Added an entry under
## Unreleasedper the convention used in the in-flight #8041 PR. (PR number placeholder#PRPENDINGwill need updating once this PR is assigned a number — happy to amend.)Notes for reviewer
TestSentryUIApplicationalready had acalledOnMainThreadtracker forwindows; the newunsafeApplicationStateReadOnMainThreadtracker follows the same pattern.dispatchSyncOnMainQueue(block:)overload also exists), happy to switch — the timeout guards against the pathological case where the caller holds a lock that main is waiting on.Closes #6591