Skip to content

[Fix] Prevent deadlocks when user manager runs its startup tasks #1559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Fix] Prevent deadlocks when user manager runs its startup tasks #1559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d32483d
Model store minimize amount of locking needed
nan-li May 12, 2025
96abd8b
Make more no-ops if there is no user instance
nan-li May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

/**
Expand Down Expand Up @@ -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)
}
}
}
10 changes: 6 additions & 4 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModelStoreListener.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 16 additions & 4 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModelStoreListener.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? ""
Expand All @@ -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 ?? ""
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading