fix: SentryAppStateManager should always stop when closing the SDK

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
 

Sources/Sentry/SentryAppStateManager.m

@@ -7,6 +7,7 @@
 #import <SentryCurrentDateProvider.h>
 #import <SentryDispatchQueueWrapper.h>
 #import <SentryFileManager.h>
+#import <SentryNSNotificationCenterWrapper.h>
 #import <SentryOptions.h>
 
 #if SENTRY_HAS_UIKIT
@@ -24,6 +25,7 @@
 @property (nonatomic, strong) id<SentryCurrentDateProvider> 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<SentryCurrentDateProvider>)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,43 +84,48 @@ - (void)start
 }
 
 - (void)stop
+{
+    [self stop:NO];
+}
+
+- (void)stop:(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];
     }
 }
 
 - (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];
 }
 
 /**

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;
     }

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 stop:YES];
+#endif
+
     // close the client
     SentryClient *client = [hub getClient];
     client.options.enabled = NO;

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<SentryCurrentDateProvider>)currentDateProvider
                          sysctl:(SentrySysctl *)sysctl
-           dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper;
+           dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper
+      notificationCenterWrapper:(SentryNSNotificationCenterWrapper *)notificationCenterWrapper;
 
 #if SENTRY_HAS_UIKIT
 
 - (void)start;
 - (void)stop;
+- (void)stop:(BOOL)forceStop;
 
 /**
  * Builds the current app state.

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(true)
+        XCTAssertEqual(sut.startCount, 0)
+
+        XCTAssertEqual(fixture.notificationCenterWrapper.removeObserverWithNameInvocations.count, 4)
+    }
+
     func testUpdateAppState() {
         sut.storeCurrentAppState()
 

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<Any>()
     public override func removeObserver(_ observer: Any) {
         removeObserverInvocations.record(observer)

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
+            )
         }
     }
 

Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift

@@ -29,15 +29,29 @@ 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)
             didFinishLaunchingTimestamp = currentDate.date().addingTimeInterval(0.3)
         }
 
         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
         }
     }

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)

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
 }