Merge pull request #1559 from OneSignal/fix/user_module_deadlocks

nan-li · web-flow · commit ba33e63a0375 · 2025-05-27T08:53:32.000-07:00
[Fix] Prevent deadlocks when user manager runs its startup tasks
diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift
@@ -105,14 +105,12 @@ open class OSModelStore<TModel: OSModel>: NSObject {
 
             // listen for changes to this model
             model.changeNotifier.subscribe(self)
-
-            guard !hydrating else {
-                return
-            }
-
-            self.changeSubscription.fire { modelStoreListener in
-                modelStoreListener.onAdded(model)
-            }
+        }
+        guard !hydrating else {
+            return
+        }
+        self.changeSubscription.fire { modelStoreListener in
+            modelStoreListener.onAdded(model)
         }
     }
 
@@ -121,24 +119,28 @@ open class OSModelStore<TModel: OSModel>: NSObject {
      This can happen if remove email or SMS is called and it doesn't exist in the store.
      */
     public func remove(_ id: String) {
+        var model: TModel?
         lock.withLock {
             OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)")
-            if let model = models[id] {
+            if let foundModel = models[id] {
+                model = foundModel
                 models.removeValue(forKey: id)
 
                 // persist the models (with removed model) to storage
                 OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
-
-                // no longer listen for changes to this model
-                model.changeNotifier.unsubscribe(self)
-
-                self.changeSubscription.fire { modelStoreListener in
-                    modelStoreListener.onRemoved(model)
-                }
             } else {
                 OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.")
+                return
             }
         }
+        guard let model = model else {
+            return
+        }
+        // no longer listen for changes to this model
+        model.changeNotifier.unsubscribe(self)
+        self.changeSubscription.fire { modelStoreListener in
+            modelStoreListener.onRemoved(model)
+        }
     }
 
     /**
@@ -167,13 +169,12 @@ extension OSModelStore: OSModelChangedHandler {
         // persist the changed models to storage
         lock.withLock {
             OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
-
-            guard !hydrating else {
-                return
-            }
-            self.changeSubscription.fire { modelStoreListener in
-                modelStoreListener.onUpdated(args)
-            }
+        }
+        guard !hydrating else {
+            return
+        }
+        self.changeSubscription.fire { modelStoreListener in
+            modelStoreListener.onUpdated(args)
         }
     }
 }
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
@@ -119,8 +119,9 @@ class OSUserExecutor {
 
             // Translate the last request into a Create User request, if the current user is the same
             if let request = transferSubscriptionRequestQueue.last,
+               let userInstance = OneSignalUserManagerImpl.sharedInstance._user,
                OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.aliasId) {
-                createUser(OneSignalUserManagerImpl.sharedInstance.user)
+                createUser(userInstance)
             }
         }
     }
@@ -396,9 +397,10 @@ extension OSUserExecutor {
 
                 self.removeFromQueue(request)
 
-                if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
+                if let userInstance = OneSignalUserManagerImpl.sharedInstance._user,
+                    OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
                     // Generate a Create User request, if it's still the current user
-                    self.createUser(OneSignalUserManagerImpl.sharedInstance.user)
+                    self.createUser(userInstance)
                 } else {
                     // This will hydrate the OneSignal ID for any pending requests
                     self.createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)
@@ -544,7 +546,7 @@ extension OSUserExecutor {
         }
 
         if let propertiesObject = parsePropertiesObjectResponse(response) {
-            OneSignalUserManagerImpl.sharedInstance.user.propertiesModel.hydrate(propertiesObject)
+            OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.hydrate(propertiesObject)
         }
 
         // Now parse email and sms subscriptions
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModelStoreListener.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModelStoreListener.swift
@@ -45,14 +45,15 @@ class OSPropertiesModelStoreListener: OSModelStoreListener {
     }
 
     func getUpdateModelDelta(_ args: OSModelChangedArgs) -> OSDelta? {
-        guard let _ = OSPropertiesSupportedProperty(rawValue: args.property) else {
-            OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertiesModelStoreListener.getUpdateModelDelta encountered unsupported property: \(args.property)")
+        guard let _ = OSPropertiesSupportedProperty(rawValue: args.property),
+              let userInstance = OneSignalUserManagerImpl.sharedInstance._user
+        else {
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertiesModelStoreListener.getUpdateModelDelta encountered unsupported property: \(args.property) or no user instance")
             return nil
         }
-
         return OSDelta(
             name: OS_UPDATE_PROPERTIES_DELTA,
-            identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
+            identityModelId: userInstance.identityModel.modelId,
             model: args.model,
             property: args.property,
             value: args.newValue
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModelStoreListener.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModelStoreListener.swift
@@ -37,9 +37,13 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
     }
 
     func getAddModelDelta(_ model: OSSubscriptionModel) -> OSDelta? {
+        guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getAddModelDelta has no user instance")
+            return nil
+        }
         return OSDelta(
             name: OS_ADD_SUBSCRIPTION_DELTA,
-            identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
+            identityModelId: userInstance.identityModel.modelId,
             model: model,
             property: model.type.rawValue, // push, email, sms
             value: model.address ?? ""
@@ -50,9 +54,13 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
      The `property` and `value` is not needed for a remove operation, so just pass in some model data as placeholders.
      */
     func getRemoveModelDelta(_ model: OSSubscriptionModel) -> OSDelta? {
+        guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getRemoveModelDelta has no user instance")
+            return nil
+        }
         return OSDelta(
             name: OS_REMOVE_SUBSCRIPTION_DELTA,
-            identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
+            identityModelId: userInstance.identityModel.modelId,
             model: model,
             property: model.type.rawValue, // push, email, sms
             value: model.address ?? ""
@@ -66,14 +74,18 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
         // The user update call increases the session_count while the subscription update would update
         // something like the app_version. If the app_version hasn't changed since the last session, there
         // wouldn't be an update needed (among other system-level properties).
-        if let onesignalId = OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId {
+        guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getUpdateModelDelta has no user instance")
+            return nil
+        }
+        if let onesignalId = userInstance.identityModel.onesignalId {
             let condition = OSIamFetchReadyCondition.sharedInstance(withId: onesignalId)
             condition.setSubscriptionUpdatePending(value: true)
         }
 
         return OSDelta(
             name: OS_UPDATE_SUBSCRIPTION_DELTA,
-            identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
+            identityModelId: userInstance.identityModel.modelId,
             model: args.model,
             property: args.property,
             value: args.newValue
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift
@@ -384,12 +384,12 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
     }
 
     func isCurrentUser(_ externalId: String) -> Bool {
-        guard !externalId.isEmpty else {
-            OneSignalLog.onesignalLog(.LL_ERROR, message: "isCurrentUser called with empty externalId")
+        guard let userInstance = _user, !externalId.isEmpty else {
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "isCurrentUser called with empty externalId or no user instance")
             return false
         }
 
-        return user.identityModel.externalId == externalId
+        return userInstance.identityModel.externalId == externalId
     }
     /**
      Clears the existing user's data in preparation for hydration via a fetch user call.

Original file line number	Diff line number	Diff line change
`@@ -119,8 +119,9 @@ class OSUserExecutor {`
`119`	`119`
`120`	`120`	`// Translate the last request into a Create User request, if the current user is the same`
`121`	`121`	`if let request = transferSubscriptionRequestQueue.last,`
	`122`	`+ let userInstance = OneSignalUserManagerImpl.sharedInstance._user,`
`122`	`123`	`OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.aliasId) {`
`123`		`- createUser(OneSignalUserManagerImpl.sharedInstance.user)`
	`124`	`+ createUser(userInstance)`
`124`	`125`	`}`
`125`	`126`	`}`
`126`	`127`	`}`
`@@ -396,9 +397,10 @@ extension OSUserExecutor {`
`396`	`397`
`397`	`398`	`self.removeFromQueue(request)`
`398`	`399`
`399`		`- if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {`
	`400`	`+ if let userInstance = OneSignalUserManagerImpl.sharedInstance._user,`
	`401`	`+ OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {`
`400`	`402`	`// Generate a Create User request, if it's still the current user`
`401`		`- self.createUser(OneSignalUserManagerImpl.sharedInstance.user)`
	`403`	`+ self.createUser(userInstance)`
`402`	`404`	`} else {`
`403`	`405`	`// This will hydrate the OneSignal ID for any pending requests`
`404`	`406`	`self.createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)`
`@@ -544,7 +546,7 @@ extension OSUserExecutor {`
`544`	`546`	`}`
`545`	`547`
`546`	`548`	`if let propertiesObject = parsePropertiesObjectResponse(response) {`
`547`		`- OneSignalUserManagerImpl.sharedInstance.user.propertiesModel.hydrate(propertiesObject)`
	`549`	`+ OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.hydrate(propertiesObject)`
`548`	`550`	`}`
`549`	`551`
`550`	`552`	`// Now parse email and sms subscriptions`
Original file line number	Diff line number	Diff line change
`@@ -384,12 +384,12 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {`
`384`	`384`	`}`
`385`	`385`
`386`	`386`	`func isCurrentUser(_ externalId: String) -> Bool {`
`387`		`- guard !externalId.isEmpty else {`
`388`		`- OneSignalLog.onesignalLog(.LL_ERROR, message: "isCurrentUser called with empty externalId")`
	`387`	`+ guard let userInstance = _user, !externalId.isEmpty else {`
	`388`	`+ OneSignalLog.onesignalLog(.LL_ERROR, message: "isCurrentUser called with empty externalId or no user instance")`
`389`	`389`	`return false`
`390`	`390`	`}`
`391`	`391`
`392`		`- return user.identityModel.externalId == externalId`
	`392`	`+ return userInstance.identityModel.externalId == externalId`
`393`	`393`	`}`
`394`	`394`	`/**`
`395`	`395`	`Clears the existing user's data in preparation for hydration via a fetch user call.`