Skip to content

Commit 3106f4d

Browse files
committed
Add lock to OSModelStore models
* The models dictionary could be mutated from multiple threads
1 parent acd3904 commit 3106f4d

File tree

1 file changed

+53
-39
lines changed

1 file changed

+53
-39
lines changed

iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ open class OSModelStore<TModel: OSModel>: NSObject {
3131
let storeKey: String
3232
let changeSubscription: OSEventProducer<OSModelStoreChangedHandler>
3333
var models: [String: TModel]
34+
let lock = NSLock()
3435

3536
public init(changeSubscription: OSEventProducer<OSModelStoreChangedHandler>, storeKey: String) {
3637
self.storeKey = storeKey
@@ -67,67 +68,76 @@ open class OSModelStore<TModel: OSModel>: NSObject {
6768
Examples: "person@example.com" for a subscription model or `OS_IDENTITY_MODEL_KEY` for an identity model.
6869
*/
6970
public func getModel(key: String) -> TModel? {
70-
return self.models[key]
71+
lock.withLock {
72+
return self.models[key]
73+
}
7174
}
7275

7376
/**
7477
Uses the `modelId` to get the corresponding model in the store's models dictionary.
7578
*/
7679
public func getModel(modelId: String) -> TModel? {
77-
for model in models.values {
78-
if model.modelId == modelId {
79-
return model
80+
lock.withLock {
81+
for model in models.values {
82+
if model.modelId == modelId {
83+
return model
84+
}
8085
}
86+
return nil
8187
}
82-
return nil
8388
}
8489

8590
public func getModels() -> [String: TModel] {
86-
return self.models
91+
lock.withLock {
92+
return self.models
93+
}
8794
}
8895

8996
public func add(id: String, model: TModel, hydrating: Bool) {
9097
// TODO: Check if we are adding the same model? Do we replace?
9198
// For example, calling addEmail multiple times with the same email
9299
// Check API endpoint for behavior
93-
models[id] = model
100+
lock.withLock {
101+
models[id] = model
94102

95-
// persist the models (including new model) to storage
96-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
103+
// persist the models (including new model) to storage
104+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
97105

98-
// listen for changes to this model
99-
model.changeNotifier.subscribe(self)
106+
// listen for changes to this model
107+
model.changeNotifier.subscribe(self)
100108

101-
guard !hydrating else {
102-
return
103-
}
109+
guard !hydrating else {
110+
return
111+
}
104112

105-
self.changeSubscription.fire { modelStoreListener in
106-
modelStoreListener.onAdded(model)
113+
self.changeSubscription.fire { modelStoreListener in
114+
modelStoreListener.onAdded(model)
115+
}
107116
}
108117
}
109118

110119
/**
111-
Returns false if this model does not exist in the store.
120+
Nothing will happen if this model does not exist in the store.
112121
This can happen if remove email or SMS is called and it doesn't exist in the store.
113122
*/
114123
public func remove(_ id: String) {
115-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)")
116-
// TODO: Nothing will happen if model doesn't exist in the store
117-
if let model = models[id] {
118-
models.removeValue(forKey: id)
119-
120-
// persist the models (with removed model) to storage
121-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
122-
123-
// no longer listen for changes to this model
124-
model.changeNotifier.unsubscribe(self)
125-
126-
self.changeSubscription.fire { modelStoreListener in
127-
modelStoreListener.onRemoved(model)
124+
lock.withLock {
125+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)")
126+
if let model = models[id] {
127+
models.removeValue(forKey: id)
128+
129+
// persist the models (with removed model) to storage
130+
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+
}
138+
} else {
139+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.")
128140
}
129-
} else {
130-
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.")
131141
}
132142
}
133143

@@ -146,20 +156,24 @@ open class OSModelStore<TModel: OSModel>: NSObject {
146156
In contrast, it is not necessary for the Identity or Properties Model Stores to do so.
147157
*/
148158
public func clearModelsFromStore() {
149-
self.models = [:]
159+
lock.withLock {
160+
self.models = [:]
161+
}
150162
}
151163
}
152164

153165
extension OSModelStore: OSModelChangedHandler {
154166
public func onModelUpdated(args: OSModelChangedArgs, hydrating: Bool) {
155167
// persist the changed models to storage
156-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
168+
lock.withLock {
169+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models)
157170

158-
guard !hydrating else {
159-
return
160-
}
161-
self.changeSubscription.fire { modelStoreListener in
162-
modelStoreListener.onUpdated(args)
171+
guard !hydrating else {
172+
return
173+
}
174+
self.changeSubscription.fire { modelStoreListener in
175+
modelStoreListener.onUpdated(args)
176+
}
163177
}
164178
}
165179
}

0 commit comments

Comments
 (0)