fix: deinit cleanup repo bad memory access (#356)

Apps/CocoaPods-FCM/Podfile

@@ -28,4 +28,6 @@ target 'Rich Push Notification Service Extension' do
   pod 'CustomerIOTracking', :path => sdk_local_path
   pod 'CustomerIOMessagingPush', :path => sdk_local_path
   pod 'CustomerIOMessagingPushFCM', :path => sdk_local_path
+
+  pod 'SampleAppsCommon', :path => File.join(File.dirname(__FILE__), '../Common')
 end 

Apps/CocoaPods-FCM/Rich Push Notification Service Extension/NotificationService.swift

@@ -1,14 +1,26 @@
 import CioMessagingPushFCM
 import CioTracking
+import SampleAppsCommon
 import UserNotifications
 
 class NotificationService: UNNotificationServiceExtension {
     override func didReceive(_ request: UNNotificationRequest, withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) {
-        // we are only using this sample app for testing it can compile so providing a siteid and apikey is not useful at the moment.
-        CustomerIO.initialize(siteId: "", apiKey: "", region: .US) { config in
-            config.logLevel = .debug
+        // This code is internal to Customer.io for testing purposes. Your app will be unique
+        // in how you decide to provide the siteId, apiKey, and region to the SDK.
+        let appSetSettings = CioSettingsManager().appSetSettings
+        let siteId = appSetSettings?.siteId ?? BuildEnvironment.CustomerIO.siteId
+        let apiKey = appSetSettings?.apiKey ?? BuildEnvironment.CustomerIO.apiKey
+
+        // You must initialize the Customer.io SDK inside of the Notification Service.
+        CustomerIO.initialize(siteId: siteId, apiKey: apiKey, region: .US) { config in
+            // Modify properties in the config object to configure the Customer.io SDK.
+            // config.logLevel = .debug // Uncomment this line to enable debug logging.
+
+            // This line of code is internal to Customer.io for testing purposes. Do not add this code to your app.
+            appSetSettings?.configureCioSdk(config: &config)
         }
 
+        // Then, send the request to the Customer.io SDK for processing.
         MessagingPush.shared.didReceive(request, withContentHandler: contentHandler)
     }
 

Apps/CocoaPods-FCM/src/AppDelegate.swift

@@ -16,6 +16,8 @@ class AppDelegate: NSObject, UIApplicationDelegate {
         // Initialize the Firebase SDK.
         FirebaseApp.configure()
 
+        // This code is internal to Customer.io for testing purposes. Your app will be unique
+        // in how you decide to provide the siteId, apiKey, and region to the SDK.
         let appSetSettings = CioSettingsManager().appSetSettings
         let siteId = appSetSettings?.siteId ?? BuildEnvironment.CustomerIO.siteId
         let apiKey = appSetSettings?.apiKey ?? BuildEnvironment.CustomerIO.apiKey
@@ -58,6 +60,22 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
     ) {
         completionHandler([.list, .banner, .badge, .sound])
     }
+
+    func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) {
+        // Send Customer.io SDK click event to process. This enables features such as
+        // push metrics and deep links.
+        let handled = MessagingPush.shared.userNotificationCenter(
+            center,
+            didReceive: response,
+            withCompletionHandler: completionHandler
+        )
+
+        // If the Customer.io SDK does not handle the push, it's up to you to handle it and call the
+        // completion handler. If the SDK did handle it, it called the completion handler for you.
+        if !handled {
+            completionHandler()
+        }
+    }
 }
 
 extension AppDelegate: InAppEventListener {

Apps/Common/Source/Model/CioSettings.swift

@@ -25,6 +25,15 @@ public struct CioSettings: Codable {
         }
     }
 
+    public func configureCioSdk(config: inout CioNotificationServiceExtensionSdkConfig) {
+        config.trackingApiUrl = trackUrl
+        config.autoTrackDeviceAttributes = trackDeviceAttributes
+
+        if debugSdkMode {
+            config.logLevel = .debug
+        }
+    }
+
     public static func getFromCioSdk() -> CioSettings {
         let sdkConfig = CustomerIO.shared.config!
 

Sources/Common/DIGraph.swift

@@ -22,6 +22,10 @@ public class DIGraph {
         overrides[String(describing: type)] = value
     }
 
+    public func resetOverride(forType type: Any.Type) {
+        overrides[String(describing: type)] = nil
+    }
+
     // Retrieves an overridden instance of a specified type from the `overrides` dictionary.
     // If an overridden instance exists and can be cast to the specified type, it is returned; otherwise, nil is returned.
     public func getOverriddenInstance<T: Any>() -> T? {

Sources/Common/Util/Lock.swift

@@ -16,11 +16,11 @@ public class Lock {
 
     private init() {}
 
-    func lock() {
+    public func lock() {
         _lock.lock()
     }
 
-    func unlock() {
+    public func unlock() {
         _lock.unlock()
     }
 }

Sources/Tracking/CustomerIO.swift

@@ -108,6 +108,11 @@ public class CustomerIO: CustomerIOInstance {
      */
     @Atomic public private(set) static var shared = CustomerIO()
 
+    // mutex to safely initialize the SDK multiple times in short amount of time.
+    // Not using LockManager because 'shared' instance will keep a singleton instance of this lock
+    // through entire lifecycle of SDK.
+    @Atomic private var initializeLock = Lock.unsafeInit()
+
     // Only assign a value to this *when the SDK is initialzied*.
     // It's assumed that if this instance is not-nil, the SDK has been initialized.
     // Tip: Use `SdkInitializedUtil` in modules to see if the SDK has been initialized and get data it needs.
@@ -117,9 +122,6 @@ public class CustomerIO: CustomerIOInstance {
     // Exposed for `SdkInitializedUtil`. Not recommended to use this property directly.
     internal var diGraph: DIGraph?
 
-    // strong reference to repository to prevent garbage collection as it runs tasks in async.
-    private var cleanupRepository: CleanupRepository?
-
     // private constructor to force use of singleton API
     private init() {}
 
@@ -143,9 +145,7 @@ public class CustomerIO: CustomerIOInstance {
     internal static func initializeIntegrationTests(
         diGraph: DIGraph
     ) {
-        let implementation = CustomerIOImplementation(diGraph: diGraph)
-        Self.shared = CustomerIO(implementation: implementation, diGraph: diGraph)
-        Self.shared.postInitialize(diGraph: diGraph)
+        Self.shared.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph))
     }
 
     /**
@@ -165,7 +165,7 @@ public class CustomerIO: CustomerIOInstance {
             configureHandler(&newSdkConfig)
         }
 
-        Self.initialize(config: newSdkConfig)
+        Self.shared.commonInitialize(config: newSdkConfig)
 
         if newSdkConfig.autoTrackScreenViews {
             // Setting up screen view tracking is not available for rich push (Notification Service Extension).
@@ -192,44 +192,51 @@ public class CustomerIO: CustomerIOInstance {
             configureHandler(&newSdkConfig)
         }
 
-        Self.initialize(config: newSdkConfig.toSdkConfig())
+        Self.shared.commonInitialize(config: newSdkConfig.toSdkConfig())
     }
 
-    // private shared logic initialize to avoid copy/paste between the different
-    // public initialize functions.
-    private static func initialize(
-        config: SdkConfig
-    ) {
+    // convenience method that takes a new config object
+    internal func commonInitialize(config: SdkConfig) {
         let newDiGraph = DIGraph(sdkConfig: config)
 
-        Self.shared.diGraph = newDiGraph
-        Self.shared.implementation = CustomerIOImplementation(diGraph: newDiGraph)
-
-        Self.shared.postInitialize(diGraph: newDiGraph)
+        commonInitialize(newDiGraph: newDiGraph, newImplementation: CustomerIOImplementation(diGraph: newDiGraph))
     }
 
-    // Contains all logic shared between all of the initialize() functions.
-    internal func postInitialize(diGraph: DIGraph) {
-        let hooks = diGraph.hooksManager
-        let threadUtil = diGraph.threadUtil
-        let logger = diGraph.logger
-        let siteId = diGraph.sdkConfig.siteId
+    // Contains all logic shared between all of the initialize() functions, including integration tests.
+    internal func commonInitialize(newDiGraph: DIGraph, newImplementation: CustomerIOImplementation) {
+        // SDK initialization could be called multiple times within a close amount of time (example: app launch in a SDK wrapper app).
+
+        // lock before setting new property values such as diGraph. Unlock after it's safe to re-set dependency graph if SDK is being initialized multiple times.
+        initializeLock.lock()
 
-        cleanupRepository = diGraph.cleanupRepository
+        diGraph = newDiGraph
+        implementation = newImplementation
+
+        let hooks = newDiGraph.hooksManager
+        let threadUtil = newDiGraph.threadUtil
+        let logger = newDiGraph.logger
+        let siteId = newDiGraph.sdkConfig.siteId
+
+        let cleanupRepository = newDiGraph.cleanupRepository
 
         // Register Tracking module hooks now that the module is being initialized.
         hooks.add(key: .tracking, provider: TrackingModuleHookProvider())
 
         // Register the device token during SDK initialization to address device registration issues
         // arising from lifecycle differences between wrapper SDKs and native SDK.
-        let globalDataStore = diGraph.globalDataStore
+        let globalDataStore = newDiGraph.globalDataStore
         if let token = globalDataStore.pushDeviceToken {
             registerDeviceToken(token)
         }
 
         // run cleanup in background to prevent locking the UI thread
-        threadUtil.runBackground { [weak self] in
-            self?.cleanupRepository?.cleanup()
+        threadUtil.runBackground {
+            // keep a strong reference to self and all dependencies in callback.
+            // memory access exceptions can occur if dependencies get deinitialized before the callback gets called.
+            cleanupRepository.cleanup()
+
+            // unlock only after it's safe for dependencies to be re-created in the SDK.
+            self.initializeLock.unlock()
         }
 
         logger
@@ -396,4 +403,4 @@ public class CustomerIO: CustomerIOInstance {
     ) {
         implementation?.trackMetric(deliveryID: deliveryID, event: event, deviceToken: deviceToken)
     }
-}
+} // swiftlint:disable:this file_length

Tests/MessagingInApp/MessagingInAppIntegrationTest.swift

@@ -9,7 +9,7 @@ class MessagingInAppIntegrationTests: IntegrationTest {
     // Bug reported when: using in-app messaging, debug logging enabled, and using iOS version 1.2.8.
     // Report: https://github.com/customerio/customerio-ios/issues/242
     func test_initialize_enableDebugLogs_assertNotCrash() {
-        tearDown() // re-initialize with different configuration options.
+        uninitializeSDK() // re-initialize with different configuration options.
 
         CustomerIO.initialize(siteId: testSiteId, apiKey: .random, region: .EU) {
             $0.logLevel = .debug

Tests/Shared/Stub/ThreadUtilStub.swift

@@ -2,11 +2,22 @@ import CioInternalCommon
 import Foundation
 
 public class ThreadUtilStub: ThreadUtil {
+    @Atomic public var runMainCallsCount = 0
+    @Atomic public var runBackgroundCallsCount = 0
+
+    public var runCount: Int {
+        runMainCallsCount + runBackgroundCallsCount
+    }
+
     public func runMain(_ block: @escaping () -> Void) {
+        runMainCallsCount += 1
+
         block()
     }
 
     public func runBackground(_ block: @escaping () -> Void) {
+        runBackgroundCallsCount += 1
+
         block()
     }
 }

Tests/Tracking/CustomerIOIntegrationTests.swift

@@ -156,4 +156,45 @@ class CustomerIOIntegrationTests: IntegrationTest {
         XCTAssertEqual(CustomerIO.shared.config!.siteId, givenSiteId)
         XCTAssertEqual(CustomerIO.shared.config!.apiKey, givenApiKey)
     }
+
+    // Reproduces edge case where SDK is initialized multiple times in a short amount of time (such as during app launch in a SDK wrapper app).
+    // Learn more: https://github.com/customerio/opsbugs/issues/3618
+    func test_initializeSdk_givenCallMultipleTimes_expectNoExceptions() {
+        // It's very important that this test runs using real background threads to make SDK initialization async.
+
+        diGraph.resetOverride(forType: ThreadUtil.self)
+        let threadRunCountBeforeTest = threadUtilStub.runCount // test class may have already initialized the SDK once. Get count now to see if it's modified after test.
+
+        // 1000 is arbitrary. From running, exception is thrown consistently between index 100 and index 500.
+        for _ in 0 ..< 1000 {
+            CustomerIO.initialize(siteId: .random, apiKey: .random, region: .US, configure: nil)
+        }
+
+        // Assert that test executed using real background threads.
+        XCTAssertEqual(threadUtilStub.runCount, threadRunCountBeforeTest)
+    }
+
+    // The SDK initialization involves async operations. Assert that the async operations all exectute and do not get skipped with multiple requests.
+    func test_initializeSdk_givenCallMultipleTimes_expectFinishAsyncInitialize() {
+        let givenNumberOfInitCalls = 100
+        let expectCleanupMultipleTimes = expectation(description: "cleanup multiple times")
+        expectCleanupMultipleTimes.expectedFulfillmentCount = givenNumberOfInitCalls
+
+        // The cleanup repository is the async operation that executes during SDK initialization.
+        // Assert that cleanup executes as many times as the SDK is initialized.
+        let cleanupRepositoryMock = CleanupRepositoryMock()
+        cleanupRepositoryMock.cleanupClosure = {
+            Thread.sleep(forTimeInterval: 0.1) // sleep for 100 milliseconds to simulate async operation
+            expectCleanupMultipleTimes.fulfill()
+        }
+
+        diGraph.resetOverride(forType: ThreadUtil.self) // use real background threads to assert we are running async operations.
+        diGraph.override(value: cleanupRepositoryMock, forType: CleanupRepository.self)
+
+        for _ in 0 ..< givenNumberOfInitCalls {
+            CustomerIO.initializeIntegrationTests(diGraph: diGraph)
+        }
+
+        waitForExpectations(timeout: 3)
+    }
 }

Tests/Tracking/CustomerIOTest.swift

@@ -23,7 +23,7 @@ class CustomerIOTest: UnitTest {
     }
 
     func test_initialize_expectAddModuleHooks_expectRunCleanup() {
-        customerIO.postInitialize(diGraph: diGraph)
+        customerIO.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph))
 
         XCTAssertEqual(hooksMock.addCallsCount, 1)
         XCTAssertEqual(hooksMock.addReceivedArguments?.key, .tracking)