Skip to content

Commit 2b15045

Browse files
committed
Synchronize the User Manager start up process
* The OneSignalUserManagerImpl `start()` method sets up initializers and local state. * Now prevent any race conditions that could occur with multiple threads trying to start up the manager simultaneously by making it thread-safe, atomic, and properly handle concurrent access. * We must now be careful of producing a deadlock situation within the start() method.
1 parent 1e02be8 commit 2b15045

File tree

1 file changed

+75
-62
lines changed

1 file changed

+75
-62
lines changed

iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift

Lines changed: 75 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
122122

123123
let newRecordsState = OSNewRecordsState()
124124

125+
private let startQueue = DispatchQueue(label: "com.onesignal.user.start")
125126
var hasCalledStart = false
126127

127128
private var jwtExpiredHandler: OSJwtExpiredHandler?
@@ -192,71 +193,78 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
192193
self.pushSubscriptionImpl = OSPushSubscriptionImpl(pushSubscriptionModelStore: pushSubscriptionModelStore)
193194
}
194195

195-
// TODO: This method is called A LOT, check if all calls are needed.
196196
@objc
197197
public func start() {
198198
guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: nil) else {
199199
return
200200
}
201-
guard !hasCalledStart else {
202-
return
203-
}
204201

205-
hasCalledStart = true
206-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager calling start")
207-
208-
OSNotificationsManager.delegate = self
209-
210-
var hasCachedUser = false
211-
212-
// Path 1. Load user from cache, if any
213-
// Corrupted state if any of these models exist without the others
214-
if let identityModel = identityModelStore.getModels()[OS_IDENTITY_MODEL_KEY],
215-
let propertiesModel = propertiesModelStore.getModels()[OS_PROPERTIES_MODEL_KEY],
216-
let pushSubscription = pushSubscriptionModelStore.getModels()[OS_PUSH_SUBSCRIPTION_MODEL_KEY] {
217-
hasCachedUser = true
218-
_user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription)
219-
addIdentityModelToRepo(identityModel)
220-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager.start called, loaded the user from cache.")
221-
}
222-
223-
// TODO: Update the push sub model with any new state from NotificationsManager
224-
225-
// Setup the executors
226-
// The OSUserExecutor has to run first, before other executors
227-
self.userExecutor = OSUserExecutor(newRecordsState: newRecordsState)
228-
OSOperationRepo.sharedInstance.start()
229-
230-
// Cannot initialize these executors in `init` as they reference the sharedInstance
231-
let propertyExecutor = OSPropertyOperationExecutor(newRecordsState: newRecordsState)
232-
let identityExecutor = OSIdentityOperationExecutor(newRecordsState: newRecordsState)
233-
let subscriptionExecutor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState)
234-
self.propertyExecutor = propertyExecutor
235-
self.identityExecutor = identityExecutor
236-
self.subscriptionExecutor = subscriptionExecutor
237-
OSOperationRepo.sharedInstance.addExecutor(identityExecutor)
238-
OSOperationRepo.sharedInstance.addExecutor(propertyExecutor)
239-
OSOperationRepo.sharedInstance.addExecutor(subscriptionExecutor)
240-
241-
// Path 2. There is a legacy player to migrate
242-
if let legacyPlayerId = OneSignalUserDefaults.initShared().getSavedString(forKey: OSUD_LEGACY_PLAYER_ID, defaultValue: nil) {
243-
OneSignalLog.onesignalLog(.LL_DEBUG, message: "OneSignalUserManager: creating user linked to legacy subscription \(legacyPlayerId)")
244-
createUserFromLegacyPlayer(legacyPlayerId)
245-
OneSignalUserDefaults.initShared().saveString(forKey: OSUD_PUSH_SUBSCRIPTION_ID, withValue: legacyPlayerId)
246-
OneSignalUserDefaults.initStandard().removeValue(forKey: OSUD_LEGACY_PLAYER_ID)
247-
OneSignalUserDefaults.initShared().removeValue(forKey: OSUD_LEGACY_PLAYER_ID)
248-
} else {
249-
// Path 3. Creates an anonymous user if there isn't one in the cache nor a legacy player
250-
createUserIfNil()
251-
}
202+
// Use a serial queue to make the start process atomic
203+
startQueue.sync {
204+
guard !hasCalledStart else {
205+
return
206+
}
207+
208+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager calling start")
209+
210+
OSNotificationsManager.delegate = self
211+
212+
var hasCachedUser = false
252213

253-
// Model store listeners subscribe to their models
254-
identityModelStoreListener.start()
255-
propertiesModelStoreListener.start()
256-
subscriptionModelStoreListener.start()
257-
pushSubscriptionModelStoreListener.start()
258-
if hasCachedUser {
259-
_user?.update()
214+
// Path 1. Load user from cache, if any
215+
// Corrupted state if any of these models exist without the others
216+
if let identityModel = identityModelStore.getModels()[OS_IDENTITY_MODEL_KEY],
217+
let propertiesModel = propertiesModelStore.getModels()[OS_PROPERTIES_MODEL_KEY],
218+
let pushSubscription = pushSubscriptionModelStore.getModels()[OS_PUSH_SUBSCRIPTION_MODEL_KEY] {
219+
hasCachedUser = true
220+
_user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription)
221+
addIdentityModelToRepo(identityModel)
222+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager.start called, loaded the user from cache.")
223+
}
224+
225+
// TODO: Update the push sub model with any new state from NotificationsManager
226+
227+
// Setup the executors
228+
// The OSUserExecutor has to run first, before other executors
229+
self.userExecutor = OSUserExecutor(newRecordsState: newRecordsState)
230+
OSOperationRepo.sharedInstance.start()
231+
232+
// Cannot initialize these executors in `init` as they reference the sharedInstance
233+
let propertyExecutor = OSPropertyOperationExecutor(newRecordsState: newRecordsState)
234+
let identityExecutor = OSIdentityOperationExecutor(newRecordsState: newRecordsState)
235+
let subscriptionExecutor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState)
236+
self.propertyExecutor = propertyExecutor
237+
self.identityExecutor = identityExecutor
238+
self.subscriptionExecutor = subscriptionExecutor
239+
OSOperationRepo.sharedInstance.addExecutor(identityExecutor)
240+
OSOperationRepo.sharedInstance.addExecutor(propertyExecutor)
241+
OSOperationRepo.sharedInstance.addExecutor(subscriptionExecutor)
242+
243+
// Path 2. There is a legacy player to migrate
244+
if let legacyPlayerId = OneSignalUserDefaults.initShared().getSavedString(forKey: OSUD_LEGACY_PLAYER_ID, defaultValue: nil) {
245+
OneSignalLog.onesignalLog(.LL_DEBUG, message: "OneSignalUserManager: creating user linked to legacy subscription \(legacyPlayerId)")
246+
createUserFromLegacyPlayer(legacyPlayerId)
247+
OneSignalUserDefaults.initShared().saveString(forKey: OSUD_PUSH_SUBSCRIPTION_ID, withValue: legacyPlayerId)
248+
OneSignalUserDefaults.initStandard().removeValue(forKey: OSUD_LEGACY_PLAYER_ID)
249+
OneSignalUserDefaults.initShared().removeValue(forKey: OSUD_LEGACY_PLAYER_ID)
250+
} else {
251+
// Path 3. Creates an anonymous user if there isn't one in the cache nor a legacy player
252+
if _user == nil {
253+
// There is no user instance, initialize a "guest user"
254+
let user = createNewUser(externalId: nil, token: nil)
255+
_user = user
256+
}
257+
}
258+
259+
// Model store listeners subscribe to their models
260+
identityModelStoreListener.start()
261+
propertiesModelStoreListener.start()
262+
subscriptionModelStoreListener.start()
263+
pushSubscriptionModelStoreListener.start()
264+
if hasCachedUser {
265+
_user?.update()
266+
}
267+
hasCalledStart = true
260268
}
261269
}
262270

@@ -303,7 +311,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
303311
Migrating a legacy player does not result in a Create User call, but certain local data should be sent manually.
304312
*/
305313
private func updateLegacyPlayer(_ user: OSUserInternal) {
306-
if let timezoneId = user.propertiesModel.timezoneId {
314+
if let timezoneId = _user?.propertiesModel.timezoneId {
307315
updatePropertiesDeltas(property: .timezone_id, value: timezoneId)
308316
}
309317
}
@@ -331,7 +339,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
331339
let newUser = setNewInternalUser(externalId: externalId, pushSubscriptionModel: pushSubscriptionModel)
332340
newUser.identityModel.jwtBearerToken = token
333341
userExecutor!.createUser(newUser)
334-
return self.user
342+
return newUser
335343
}
336344

337345
/**
@@ -478,8 +486,9 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
478486
pushSubscriptionModelStore.add(id: OS_PUSH_SUBSCRIPTION_MODEL_KEY, model: pushSubscription, hydrating: false)
479487
}
480488

481-
_user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription)
482-
return self.user
489+
let newUser = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription)
490+
_user = newUser
491+
return newUser
483492
}
484493

485494
/**
@@ -585,6 +594,10 @@ extension OneSignalUserManagerImpl {
585594
return
586595
}
587596

597+
guard let user = _user else {
598+
return
599+
}
600+
588601
// Get the identity and properties model of the current user
589602
let identityModel = user.identityModel
590603
let propertiesModel = user.propertiesModel

0 commit comments

Comments
 (0)