diff --git a/CHANGELOG.md b/CHANGELOG.md index 24c51f2395f..21c41a74370 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Always start SDK on the main thread (#3291) - App hang with race condition for tick counter (#3290) - Remove "duplicate library" warning (#3312) - Remove unnecessary build settings (#3325) diff --git a/Sources/Sentry/Public/SentrySDK.h b/Sources/Sentry/Public/SentrySDK.h index 344eb1ffe0b..94aee0e776e 100644 --- a/Sources/Sentry/Public/SentrySDK.h +++ b/Sources/Sentry/Public/SentrySDK.h @@ -27,12 +27,18 @@ SENTRY_NO_INIT /** * Inits and configures Sentry (SentryHub, SentryClient) and sets up all integrations. Make sure to * set a valid DSN. + * + * @discussion Call this method on the main thread. When calling it from a background thread, the + * SDK starts on the main thread async. */ + (void)startWithOptions:(SentryOptions *)options NS_SWIFT_NAME(start(options:)); /** * Inits and configures Sentry (SentryHub, SentryClient) and sets up all integrations. Make sure to * set a valid DSN. + * + * @discussion Call this method on the main thread. When calling it from a background thread, the + * SDK starts on the main thread async. */ + (void)startWithConfigureOptions:(void (^)(SentryOptions *options))configureOptions NS_SWIFT_NAME(start(configureOptions:)); diff --git a/Sources/Sentry/SentryReachability.m b/Sources/Sentry/SentryReachability.m index 3157f10cbdb..7c1d3398284 100644 --- a/Sources/Sentry/SentryReachability.m +++ b/Sources/Sentry/SentryReachability.m @@ -98,6 +98,13 @@ } } +void +SentryConnectivityReset(void) +{ + [sentry_reachability_change_blocks removeAllObjects]; + sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized; +} + @implementation SentryReachability + (void)initialize @@ -109,9 +116,8 @@ + (void)initialize - (void)dealloc { - for (id observer in sentry_reachability_change_blocks.allKeys) { - [self removeObserver:observer]; - } + sentry_reachability_change_blocks = [NSMutableDictionary dictionary]; + [self removeReachabilityNotification]; } - (void)addObserver:(id)observer @@ -147,6 +153,11 @@ - (void)removeObserver:(id)observer return; } + [self removeReachabilityNotification]; +} + +- (void)removeReachabilityNotification +{ sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized; if (_sentry_reachability_ref == nil) { diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 91e7e104227..c81bf2ef219 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -8,12 +8,14 @@ #import "SentryCrash.h" #import "SentryCrashWrapper.h" #import "SentryDependencyContainer.h" +#import "SentryDispatchQueueWrapper.h" #import "SentryFileManager.h" #import "SentryHub+Private.h" #import "SentryLog.h" #import "SentryMeta.h" #import "SentryOptions+Private.h" #import "SentryScope.h" +#import "SentryThreadWrapper.h" #import "SentryUIDeviceWrapper.h" @interface @@ -135,27 +137,32 @@ + (void)setStartInvocations:(NSUInteger)value + (void)startWithOptions:(SentryOptions *)options { - startInvocations++; + // We accept the tradeoff that the SDK might not be fully initialized directly after + // initializing it on a background thread because scheduling the init synchronously on the main + // thread could lead to deadlocks. + [SentryThreadWrapper onMainThread:^{ + startInvocations++; - [SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel]; + [SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel]; - SentryClient *newClient = [[SentryClient alloc] initWithOptions:options]; - [newClient.fileManager moveAppStateToPreviousAppState]; - [newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs]; + SentryClient *newClient = [[SentryClient alloc] initWithOptions:options]; + [newClient.fileManager moveAppStateToPreviousAppState]; + [newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs]; - SentryScope *scope - = options.initialScope([[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]); - // The Hub needs to be initialized with a client so that closing a session - // can happen. - [SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]]; - SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString); - [SentrySDK installIntegrations]; + SentryScope *scope = options.initialScope( + [[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]); + // The Hub needs to be initialized with a client so that closing a session + // can happen. + [SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]]; + SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString); + [SentrySDK installIntegrations]; - [SentryCrashWrapper.sharedInstance startBinaryImageCache]; - [SentryDependencyContainer.sharedInstance.binaryImageCache start]; + [SentryCrashWrapper.sharedInstance startBinaryImageCache]; + [SentryDependencyContainer.sharedInstance.binaryImageCache start]; #if TARGET_OS_IOS - [SentryDependencyContainer.sharedInstance.uiDeviceWrapper start]; + [SentryDependencyContainer.sharedInstance.uiDeviceWrapper start]; #endif + }]; } + (void)startWithConfigureOptions:(void (^)(SentryOptions *options))configureOptions diff --git a/Tests/SentryTests/Networking/SentryReachabilityTests.m b/Tests/SentryTests/Networking/SentryReachabilityTests.m index bf733289d0a..5e729343127 100644 --- a/Tests/SentryTests/Networking/SentryReachabilityTests.m +++ b/Tests/SentryTests/Networking/SentryReachabilityTests.m @@ -2,6 +2,8 @@ #import "SentryReachability.h" #import +void SentryConnectivityReset(void); + @interface TestSentryReachabilityObserver : NSObject @end @implementation TestSentryReachabilityObserver @@ -22,6 +24,7 @@ - (void)setUp - (void)tearDown { self.reachability = nil; + SentryConnectivityReset(); } - (void)testConnectivityRepresentations diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 7fd69ab89b3..3ad7532d5cf 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -653,7 +653,25 @@ class SentrySDKTests: XCTestCase { XCTAssertEqual(flushTimeout, transport.flushInvocations.first) } - + + func testStartOnTheMainThread() { + + let expectation = expectation(description: "MainThreadTestIntegration install called") + MainThreadTestIntegration.expectation = expectation + + DispatchQueue.global(qos: .background).async { + SentrySDK.start { options in + options.integrations = [ NSStringFromClass(MainThreadTestIntegration.self) ] + } + } + + wait(for: [expectation], timeout: 1.0) + + let mainThreadIntegration = SentrySDK.currentHub().installedIntegrations().first { integration in integration is MainThreadTestIntegration } as? MainThreadTestIntegration + XCTAssertEqual(mainThreadIntegration?.installedInTheMainThread, true, "SDK is not being initialized in the main thread") + + } + #if SENTRY_HAS_UIKIT func testSetAppStartMeasurementConcurrently() { @@ -773,3 +791,17 @@ class SentrySDKTests: XCTestCase { fixture.currentDate.setDate(date: SentryDependencyContainer.sharedInstance().dateProvider.date().addingTimeInterval(bySeconds)) } } + +public class MainThreadTestIntegration: NSObject, SentryIntegrationProtocol { + + static var expectation: XCTestExpectation? + + public var installedInTheMainThread = false + + public func install(with options: Options) -> Bool { + installedInTheMainThread = Thread.isMainThread + MainThreadTestIntegration.expectation?.fulfill() + MainThreadTestIntegration.expectation = nil + return true + } +} diff --git a/develop-docs/DECISIONS.md b/develop-docs/DECISIONS.md index 119e2e8d87b..26b10ca758c 100644 --- a/develop-docs/DECISIONS.md +++ b/develop-docs/DECISIONS.md @@ -127,3 +127,16 @@ This way, if these files are changed, we will be reminded to test the changes wi Additionally, two new 'make' commands(test-alamofire, test-homekit) are being added to the project to assist in testing the Sentry SDK in third-party projects. Related to [GH-2916](https://github.com/getsentry/sentry-cocoa/pull/2916) + +## Async SDK init on main thread + +Date: October 11th 2023 +Contributors: @philipphofmann, @brustolin + +We decided to initialize the SDK on the main thread async when being initialized from a background thread. +We accept the tradeoff that the SDK might not be fully initialized directly after initializing it on a background +thread because scheduling the init synchronously on the main thread could lead to deadlocks, such as https://github.com/getsentry/sentry-cocoa/issues/3277. + +Related links: + +- https://github.com/getsentry/sentry-cocoa/pull/3291