Skip to content

Commit ba33e63

Browse files
nan-linan-li
authored
Merge pull request #1559 from OneSignal/fix/user_module_deadlocks
[Fix] Prevent deadlocks when user manager runs its startup tasks
2 parents 3af1ea8 + 96abd8b commit ba33e63

File tree

5 files changed

+54
-38
lines changed

5 files changed

+54
-38
lines changed

‎iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,12 @@ open class OSModelStore<TModel: OSModel>: NSObject {
105105

106106
// listen for changes to this model
107107
model.changeNotifier.subscribe(self)
108-
109-
guard !hydrating else {
110-
return
111-
}
112-
113-
self.changeSubscription.fire { modelStoreListener in
114-
modelStoreListener.onAdded(model)
115-
}
108+
}
109+
guard !hydrating else {
110+
return
111+
}
112+
self.changeSubscription.fire { modelStoreListener in
113+
modelStoreListener.onAdded(model)
116114
}
117115
}
118116

@@ -121,24 +119,28 @@ open class OSModelStore<TModel: OSModel>: NSObject {
121119
This can happen if remove email or SMS is called and it doesn't exist in the store.
122120
*/
123121
public func remove(_ id: String) {
122+
var model: TModel?
124123
lock.withLock {
125124
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)")
126-
if let model = models[id] {
125+
if let foundModel = models[id] {
126+
model = foundModel
127127
models.removeValue(forKey: id)
128128

129129
// persist the models (with removed model) to storage
130130
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
131-
132-
// no longer listen for changes to this model
133-
model.changeNotifier.unsubscribe(self)
134-
135-
self.changeSubscription.fire { modelStoreListener in
136-
modelStoreListener.onRemoved(model)
137-
}
138131
} else {
139132
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.")
133+
return
140134
}
141135
}
136+
guard let model = model else {
137+
return
138+
}
139+
// no longer listen for changes to this model
140+
model.changeNotifier.unsubscribe(self)
141+
self.changeSubscription.fire { modelStoreListener in
142+
modelStoreListener.onRemoved(model)
143+
}
142144
}
143145

144146
/**
@@ -167,13 +169,12 @@ extension OSModelStore: OSModelChangedHandler {
167169
// persist the changed models to storage
168170
lock.withLock {
169171
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
170-
171-
guard !hydrating else {
172-
return
173-
}
174-
self.changeSubscription.fire { modelStoreListener in
175-
modelStoreListener.onUpdated(args)
176-
}
172+
}
173+
guard !hydrating else {
174+
return
175+
}
176+
self.changeSubscription.fire { modelStoreListener in
177+
modelStoreListener.onUpdated(args)
177178
}
178179
}
179180
}

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,9 @@ class OSUserExecutor {
119119

120120
// Translate the last request into a Create User request, if the current user is the same
121121
if let request = transferSubscriptionRequestQueue.last,
122+
let userInstance = OneSignalUserManagerImpl.sharedInstance._user,
122123
OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.aliasId) {
123-
createUser(OneSignalUserManagerImpl.sharedInstance.user)
124+
createUser(userInstance)
124125
}
125126
}
126127
}
@@ -396,9 +397,10 @@ extension OSUserExecutor {
396397

397398
self.removeFromQueue(request)
398399

399-
if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
400+
if let userInstance = OneSignalUserManagerImpl.sharedInstance._user,
401+
OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
400402
// Generate a Create User request, if it's still the current user
401-
self.createUser(OneSignalUserManagerImpl.sharedInstance.user)
403+
self.createUser(userInstance)
402404
} else {
403405
// This will hydrate the OneSignal ID for any pending requests
404406
self.createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)
@@ -544,7 +546,7 @@ extension OSUserExecutor {
544546
}
545547

546548
if let propertiesObject = parsePropertiesObjectResponse(response) {
547-
OneSignalUserManagerImpl.sharedInstance.user.propertiesModel.hydrate(propertiesObject)
549+
OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.hydrate(propertiesObject)
548550
}
549551

550552
// Now parse email and sms subscriptions

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModelStoreListener.swift‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ class OSPropertiesModelStoreListener: OSModelStoreListener {
4545
}
4646

4747
func getUpdateModelDelta(_ args: OSModelChangedArgs) -> OSDelta? {
48-
guard let _ = OSPropertiesSupportedProperty(rawValue: args.property) else {
49-
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertiesModelStoreListener.getUpdateModelDelta encountered unsupported property: \(args.property)")
48+
guard let _ = OSPropertiesSupportedProperty(rawValue: args.property),
49+
let userInstance = OneSignalUserManagerImpl.sharedInstance._user
50+
else {
51+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertiesModelStoreListener.getUpdateModelDelta encountered unsupported property: \(args.property) or no user instance")
5052
return nil
5153
}
52-
5354
return OSDelta(
5455
name: OS_UPDATE_PROPERTIES_DELTA,
55-
identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
56+
identityModelId: userInstance.identityModel.modelId,
5657
model: args.model,
5758
property: args.property,
5859
value: args.newValue

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModelStoreListener.swift‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
3737
}
3838

3939
func getAddModelDelta(_ model: OSSubscriptionModel) -> OSDelta? {
40+
guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
41+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getAddModelDelta has no user instance")
42+
return nil
43+
}
4044
return OSDelta(
4145
name: OS_ADD_SUBSCRIPTION_DELTA,
42-
identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
46+
identityModelId: userInstance.identityModel.modelId,
4347
model: model,
4448
property: model.type.rawValue, // push, email, sms
4549
value: model.address ?? ""
@@ -50,9 +54,13 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
5054
The `property` and `value` is not needed for a remove operation, so just pass in some model data as placeholders.
5155
*/
5256
func getRemoveModelDelta(_ model: OSSubscriptionModel) -> OSDelta? {
57+
guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
58+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getRemoveModelDelta has no user instance")
59+
return nil
60+
}
5361
return OSDelta(
5462
name: OS_REMOVE_SUBSCRIPTION_DELTA,
55-
identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
63+
identityModelId: userInstance.identityModel.modelId,
5664
model: model,
5765
property: model.type.rawValue, // push, email, sms
5866
value: model.address ?? ""
@@ -66,14 +74,18 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener {
6674
// The user update call increases the session_count while the subscription update would update
6775
// something like the app_version. If the app_version hasn't changed since the last session, there
6876
// wouldn't be an update needed (among other system-level properties).
69-
if let onesignalId = OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId {
77+
guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else {
78+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getUpdateModelDelta has no user instance")
79+
return nil
80+
}
81+
if let onesignalId = userInstance.identityModel.onesignalId {
7082
let condition = OSIamFetchReadyCondition.sharedInstance(withId: onesignalId)
7183
condition.setSubscriptionUpdatePending(value: true)
7284
}
7385

7486
return OSDelta(
7587
name: OS_UPDATE_SUBSCRIPTION_DELTA,
76-
identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
88+
identityModelId: userInstance.identityModel.modelId,
7789
model: args.model,
7890
property: args.property,
7991
value: args.newValue

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,12 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
384384
}
385385

386386
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")
389389
return false
390390
}
391391

392-
return user.identityModel.externalId == externalId
392+
return userInstance.identityModel.externalId == externalId
393393
}
394394
/**
395395
Clears the existing user's data in preparation for hydration via a fetch user call.

0 commit comments

Comments
 (0)