diff --git a/CHANGELOG.md b/CHANGELOG.md index cf6a8ddf300..359f2380572 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,9 @@ This version adds a dependency on Swift. ### Fixes -- Errors shortly after SentrySDK.init now affect the session (#2430) +- Errors shortly after `SentrySDK.init` now affect the session (#2430) - Use the same default environment for events and sessions (#2447) +- `SentryAppStateManager` correctly unsubscribes from `NSNotificationCenter` when closing the SDK (#2460) ### Breaking Changes diff --git a/Sources/Sentry/SentryAppStateManager.m b/Sources/Sentry/SentryAppStateManager.m index 0302320f5e1..7c2d783cb18 100644 --- a/Sources/Sentry/SentryAppStateManager.m +++ b/Sources/Sentry/SentryAppStateManager.m @@ -7,6 +7,7 @@ #import #import #import +#import #import #if SENTRY_HAS_UIKIT @@ -24,6 +25,7 @@ @property (nonatomic, strong) id currentDate; @property (nonatomic, strong) SentrySysctl *sysctl; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue; +@property (nonatomic, strong) SentryNSNotificationCenterWrapper *notificationCenterWrapper; @property (nonatomic) NSInteger startCount; @end @@ -36,6 +38,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options currentDateProvider:(id)currentDateProvider sysctl:(SentrySysctl *)sysctl dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper + notificationCenterWrapper:(SentryNSNotificationCenterWrapper *)notificationCenterWrapper { if (self = [super init]) { self.options = options; @@ -44,6 +47,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options self.currentDate = currentDateProvider; self.sysctl = sysctl; self.dispatchQueue = dispatchQueueWrapper; + self.notificationCenterWrapper = notificationCenterWrapper; self.startCount = 0; } return self; @@ -54,29 +58,24 @@ - (instancetype)initWithOptions:(SentryOptions *)options - (void)start { if (self.startCount == 0) { - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper addObserver:self selector:@selector(didBecomeActive) - name:SentryNSNotificationCenterWrapper.didBecomeActiveNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.didBecomeActiveNotificationName]; - [NSNotificationCenter.defaultCenter - addObserver:self - selector:@selector(didBecomeActive) - name:SentryHybridSdkDidBecomeActiveNotificationName - object:nil]; + [self.notificationCenterWrapper addObserver:self + selector:@selector(didBecomeActive) + name:SentryHybridSdkDidBecomeActiveNotificationName]; - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper addObserver:self selector:@selector(willResignActive) - name:SentryNSNotificationCenterWrapper.willResignActiveNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.willResignActiveNotificationName]; - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper addObserver:self selector:@selector(willTerminate) - name:SentryNSNotificationCenterWrapper.willTerminateNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.willTerminateNotificationName]; [self storeCurrentAppState]; } @@ -85,35 +84,40 @@ - (void)start } - (void)stop +{ + [self stopWithForce:NO]; +} + +- (void)stopWithForce:(BOOL)forceStop { if (self.startCount <= 0) { return; } - self.startCount -= 1; + if (forceStop) { + self.startCount = 0; + } else { + self.startCount -= 1; + } if (self.startCount == 0) { // Remove the observers with the most specific detail possible, see // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper removeObserver:self - name:SentryNSNotificationCenterWrapper.didBecomeActiveNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.didBecomeActiveNotificationName]; - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper removeObserver:self - name:SentryHybridSdkDidBecomeActiveNotificationName - object:nil]; + name:SentryHybridSdkDidBecomeActiveNotificationName]; - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper removeObserver:self - name:SentryNSNotificationCenterWrapper.willResignActiveNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.willResignActiveNotificationName]; - [NSNotificationCenter.defaultCenter + [self.notificationCenterWrapper removeObserver:self - name:SentryNSNotificationCenterWrapper.willTerminateNotificationName - object:nil]; + name:SentryNSNotificationCenterWrapper.willTerminateNotificationName]; } } @@ -121,7 +125,7 @@ - (void)dealloc { // In dealloc it's safe to unsubscribe for all, see // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver - [NSNotificationCenter.defaultCenter removeObserver:self]; + [self.notificationCenterWrapper removeObserver:self]; } /** diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index 17df37182b1..1cf121063f0 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -63,12 +63,13 @@ - (SentryAppStateManager *)appStateManager if (_appStateManager == nil) { SentryOptions *options = [[[SentrySDK currentHub] getClient] options]; _appStateManager = [[SentryAppStateManager alloc] - initWithOptions:options - crashWrapper:self.crashWrapper - fileManager:self.fileManager - currentDateProvider:[SentryDefaultCurrentDateProvider sharedInstance] - sysctl:[[SentrySysctl alloc] init] - dispatchQueueWrapper:self.dispatchQueueWrapper]; + initWithOptions:options + crashWrapper:self.crashWrapper + fileManager:self.fileManager + currentDateProvider:[SentryDefaultCurrentDateProvider sharedInstance] + sysctl:[[SentrySysctl alloc] init] + dispatchQueueWrapper:self.dispatchQueueWrapper + notificationCenterWrapper:self.notificationCenterWrapper]; } return _appStateManager; } diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 2d036a9bb16..2c586b632d0 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -1,6 +1,7 @@ #import "SentrySDK.h" #import "PrivateSentrySDKOnly.h" #import "SentryAppStartMeasurement.h" +#import "SentryAppStateManager.h" #import "SentryBreadcrumb.h" #import "SentryClient+Private.h" #import "SentryCrash.h" @@ -403,6 +404,12 @@ + (void)close } [hub removeAllIntegrations]; +#if SENTRY_HAS_UIKIT + // force the AppStateManager to unsubscribe, see + // https://github.com/getsentry/sentry-cocoa/issues/2455 + [[SentryDependencyContainer sharedInstance].appStateManager stopWithForce:YES]; +#endif + // close the client SentryClient *client = [hub getClient]; client.options.enabled = NO; diff --git a/Sources/Sentry/include/SentryAppStateManager.h b/Sources/Sentry/include/SentryAppStateManager.h index d50ab4ff1ad..c1cb0017d10 100644 --- a/Sources/Sentry/include/SentryAppStateManager.h +++ b/Sources/Sentry/include/SentryAppStateManager.h @@ -2,24 +2,28 @@ #import "SentryDefines.h" @class SentryOptions, SentryCrashWrapper, SentryAppState, SentryFileManager, SentrySysctl, - SentryDispatchQueueWrapper; + SentryDispatchQueueWrapper, SentryNSNotificationCenterWrapper; NS_ASSUME_NONNULL_BEGIN @interface SentryAppStateManager : NSObject SENTRY_NO_INIT +@property (nonatomic, readonly) NSInteger startCount; + - (instancetype)initWithOptions:(SentryOptions *)options crashWrapper:(SentryCrashWrapper *)crashWrapper fileManager:(SentryFileManager *)fileManager currentDateProvider:(id)currentDateProvider sysctl:(SentrySysctl *)sysctl - dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper; + dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper + notificationCenterWrapper:(SentryNSNotificationCenterWrapper *)notificationCenterWrapper; #if SENTRY_HAS_UIKIT - (void)start; - (void)stop; +- (void)stopWithForce:(BOOL)forceStop; /** * Builds the current app state. diff --git a/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift b/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift index ac98454fa9d..11e9f899b8a 100644 --- a/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryAppStateManagerTests.swift @@ -11,6 +11,7 @@ class SentryAppStateManagerTests: XCTestCase { let fileManager: SentryFileManager let currentDate = TestCurrentDateProvider() let dispatchQueue = TestSentryDispatchQueueWrapper() + let notificationCenterWrapper = TestNSNotificationCenterWrapper() init() { options = Options() @@ -27,7 +28,8 @@ class SentryAppStateManagerTests: XCTestCase { fileManager: fileManager, currentDateProvider: currentDate, sysctl: TestSysctl(), - dispatchQueueWrapper: TestSentryDispatchQueueWrapper() + dispatchQueueWrapper: TestSentryDispatchQueueWrapper(), + notificationCenterWrapper: notificationCenterWrapper ) } } @@ -76,6 +78,22 @@ class SentryAppStateManagerTests: XCTestCase { XCTAssertNotNil(fixture.fileManager.readAppState()) } + func testForcedStop() { + XCTAssertNil(fixture.fileManager.readAppState()) + + sut.start() + sut.start() + sut.start() + + sut.stop() + XCTAssertEqual(sut.startCount, 2) + + sut.stop(withForce: true) + XCTAssertEqual(sut.startCount, 0) + + XCTAssertEqual(fixture.notificationCenterWrapper.removeObserverWithNameInvocations.count, 4) + } + func testUpdateAppState() { sut.storeCurrentAppState() diff --git a/Tests/SentryTests/Helper/TestNSNotificationCenterWrapper.swift b/Tests/SentryTests/Helper/TestNSNotificationCenterWrapper.swift index 81289540ce1..1d9fc0e2f67 100644 --- a/Tests/SentryTests/Helper/TestNSNotificationCenterWrapper.swift +++ b/Tests/SentryTests/Helper/TestNSNotificationCenterWrapper.swift @@ -1,25 +1,20 @@ import Foundation @objc public class TestNSNotificationCenterWrapper: SentryNSNotificationCenterWrapper { - var addObserverInvocations = Invocations<(observer: Any, selector: Selector, name: NSNotification.Name)>() @objc public var addObserverInvocationsCount: Int { return addObserverInvocations.count } - + public override func addObserver(_ observer: Any, selector aSelector: Selector, name aName: NSNotification.Name) { addObserverInvocations.record((observer, aSelector, aName)) } - - var addObserverWithNotificationInvocations = Invocations<(observer: Any, name: NSNotification.Name)>() - @objc public var removeWithNotificationInvocationsCount: Int { - return addObserverWithNotificationInvocations.count - } - + + var removeObserverWithNameInvocations = Invocations<(observer: Any, name: NSNotification.Name)>() public override func removeObserver(_ observer: Any, name aName: NSNotification.Name) { - addObserverWithNotificationInvocations.record((observer, aName)) + removeObserverWithNameInvocations.record((observer, aName)) } - + var removeObserverInvocations = Invocations() public override func removeObserver(_ observer: Any) { removeObserverInvocations.record(observer) diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift index 05ea929fdf3..960036bf200 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift @@ -36,9 +36,27 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { } func getSut(fileManager: SentryFileManager) -> SentryOutOfMemoryTracker { - let appStateManager = SentryAppStateManager(options: options, crashWrapper: crashWrapper, fileManager: fileManager, currentDateProvider: currentDate, sysctl: sysctl, dispatchQueueWrapper: self.dispatchQueue) - let logic = SentryOutOfMemoryLogic(options: options, crashAdapter: crashWrapper, appStateManager: appStateManager) - return SentryOutOfMemoryTracker(options: options, outOfMemoryLogic: logic, appStateManager: appStateManager, dispatchQueueWrapper: dispatchQueue, fileManager: fileManager) + let appStateManager = SentryAppStateManager( + options: options, + crashWrapper: crashWrapper, + fileManager: fileManager, + currentDateProvider: currentDate, + sysctl: sysctl, + dispatchQueueWrapper: self.dispatchQueue, + notificationCenterWrapper: SentryNSNotificationCenterWrapper() + ) + let logic = SentryOutOfMemoryLogic( + options: options, + crashAdapter: crashWrapper, + appStateManager: appStateManager + ) + return SentryOutOfMemoryTracker( + options: options, + outOfMemoryLogic: logic, + appStateManager: appStateManager, + dispatchQueueWrapper: dispatchQueue, + fileManager: fileManager + ) } } diff --git a/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift index 25cdf277a6a..21557232de9 100644 --- a/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift @@ -29,7 +29,15 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase { fileManager = try! SentryFileManager(options: options, andCurrentDateProvider: currentDate, dispatchQueueWrapper: dispatchQueue) - appStateManager = SentryAppStateManager(options: options, crashWrapper: crashWrapper, fileManager: fileManager, currentDateProvider: currentDate, sysctl: sysctl, dispatchQueueWrapper: dispatchQueue) + appStateManager = SentryAppStateManager( + options: options, + crashWrapper: crashWrapper, + fileManager: fileManager, + currentDateProvider: currentDate, + sysctl: sysctl, + dispatchQueueWrapper: dispatchQueue, + notificationCenterWrapper: SentryNSNotificationCenterWrapper() + ) runtimeInitTimestamp = currentDate.date().addingTimeInterval(0.2) moduleInitializationTimestamp = currentDate.date().addingTimeInterval(0.1) @@ -37,7 +45,13 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase { } var sut: SentryAppStartTracker { - let sut = SentryAppStartTracker(currentDateProvider: currentDate, dispatchQueueWrapper: TestSentryDispatchQueueWrapper(), appStateManager: appStateManager, sysctl: sysctl, enablePreWarmedAppStartTracing: enablePreWarmedAppStartTracing) + let sut = SentryAppStartTracker( + currentDateProvider: currentDate, + dispatchQueueWrapper: TestSentryDispatchQueueWrapper(), + appStateManager: appStateManager, + sysctl: sysctl, + enablePreWarmedAppStartTracing: enablePreWarmedAppStartTracing + ) return sut } } diff --git a/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift b/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift index ad4060b447a..c9b42f0777e 100644 --- a/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift @@ -353,7 +353,7 @@ class SentrySessionTrackerTests: XCTestCase { func testStop_RemovesObservers() { sut.stop() - let invocations = fixture.notificationCenter.addObserverWithNotificationInvocations + let invocations = fixture.notificationCenter.removeObserverWithNameInvocations let notificationNames = invocations.invocations.map { $0.name } assertNotificationNames(notificationNames) diff --git a/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m b/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m index 31916f58dfb..df94165d163 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m +++ b/Tests/SentryTests/SentryCrash/SentryCrashInstallationTests.m @@ -62,7 +62,7 @@ - (void)testUninstall_CallsRemoveObservers [installation uninstall]; #if SentryCrashCRASH_HAS_UIAPPLICATION - XCTAssertEqual(5, self.notificationCenter.removeWithNotificationInvocationsCount); + XCTAssertEqual(5, self.notificationCenter.removeObserverWithNameInvocations.count); #endif } @@ -95,7 +95,7 @@ - (void)testUninstall_Install crashHandlerDataAfterInstall:crashHandlerDataAfterInstall]; #if SentryCrashCRASH_HAS_UIAPPLICATION - XCTAssertEqual(55, self.notificationCenter.removeWithNotificationInvocationsCount); + XCTAssertEqual(55, self.notificationCenter.removeObserverWithNameInvocations.count); #endif } diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 20126473576..35a001610fa 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -492,6 +492,29 @@ class SentrySDKTests: XCTestCase { XCTAssertEqual(0, hub.installedIntegrations.count) assertIntegrationsInstalled(integrations: []) } + +#if SENTRY_HAS_UIKIT + func testClose_StopsAppStateManager() { + SentrySDK.start { options in + options.dsn = SentrySDKTests.dsnAsString + options.tracesSampleRate = 1 + } + + let appStateManager = SentryDependencyContainer.sharedInstance().appStateManager + XCTAssertEqual(appStateManager.startCount, 1) + + SentrySDK.start { options in + options.dsn = SentrySDKTests.dsnAsString + options.tracesSampleRate = 1 + } + + XCTAssertEqual(appStateManager.startCount, 2) + + SentrySDK.close() + + XCTAssertEqual(appStateManager.startCount, 0) + } +#endif func testFlush_CallsFlushCorrectlyOnTransport() { SentrySDK.start { options in