Skip to content

Commit cb88aa6

Browse files
committed
Property Executor: Add private dispatch queue
* Reproduced crashes by creating multiple async threads that added and removed tags concurrently. * Added a private dispatch queue to synchronize access to the delta queue and request queue. * Crashes no longer happened after this change. * It is possible for the executor to be flushing while a client response is received and modify the request queue. * Additionally, there are some code paths that enqueue and update request but does not go through the operation repo, such as updating session count at the start of a new session.
1 parent 3183463 commit cb88aa6

File tree

1 file changed

+68
-52
lines changed

1 file changed

+68
-52
lines changed

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
3333
var deltaQueue: [OSDelta] = []
3434
var updateRequestQueue: [OSRequestUpdateProperties] = []
3535

36+
// The property executor dispatch queue, serial. This synchronizes access to `deltaQueue` and `updateRequestQueue`.
37+
private let dispatchQueue = DispatchQueue(label: "OneSignal.OSPropertyOperationExecutor", target: .global())
38+
3639
init() {
3740
// Read unfinished deltas from cache, if any...
3841
// Note that we should only have deltas for the current user as old ones are flushed..
@@ -78,42 +81,49 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
7881
}
7982

8083
func enqueueDelta(_ delta: OSDelta) {
81-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)")
82-
deltaQueue.append(delta)
84+
self.dispatchQueue.async {
85+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)")
86+
self.deltaQueue.append(delta)
87+
}
8388
}
8489

8590
func cacheDeltaQueue() {
86-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue)
91+
self.dispatchQueue.async {
92+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue)
93+
}
8794
}
8895

8996
func processDeltaQueue(inBackground: Bool) {
90-
if !deltaQueue.isEmpty {
91-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(deltaQueue)")
92-
}
93-
for delta in deltaQueue {
94-
guard let model = delta.model as? OSPropertiesModel else {
95-
// Log error
96-
continue
97+
self.dispatchQueue.async {
98+
if !self.deltaQueue.isEmpty {
99+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")
97100
}
101+
for delta in self.deltaQueue {
102+
guard let model = delta.model as? OSPropertiesModel else {
103+
// Log error
104+
continue
105+
}
98106

99-
let request = OSRequestUpdateProperties(
100-
properties: [delta.property: delta.value],
101-
deltas: nil,
102-
refreshDeviceMetadata: false, // Sort this out.
103-
modelToUpdate: model,
104-
identityModel: OneSignalUserManagerImpl.sharedInstance.user.identityModel // TODO: Make sure this is ok
105-
)
106-
updateRequestQueue.append(request)
107-
}
108-
self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue
107+
let request = OSRequestUpdateProperties(
108+
properties: [delta.property: delta.value],
109+
deltas: nil,
110+
refreshDeviceMetadata: false, // Sort this out.
111+
modelToUpdate: model,
112+
identityModel: OneSignalUserManagerImpl.sharedInstance.user.identityModel // TODO: Make sure this is ok
113+
)
114+
self.updateRequestQueue.append(request)
115+
}
116+
self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue
109117

110-
// persist executor's requests (including new request) to storage
111-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
118+
// persist executor's requests (including new request) to storage
119+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
112120

113-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) // This should be empty, can remove instead?
114-
processRequestQueue(inBackground: inBackground)
121+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) // This should be empty, can remove instead?
122+
self.processRequestQueue(inBackground: inBackground)
123+
}
115124
}
116125

126+
// This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue.
117127
func processRequestQueue(inBackground: Bool) {
118128
if updateRequestQueue.isEmpty {
119129
return
@@ -141,38 +151,42 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
141151
OneSignalCore.sharedClient().execute(request) { _ in
142152
// On success, remove request from cache, and we do need to hydrate
143153
// TODO: We need to hydrate after all ? What why ?
144-
self.updateRequestQueue.removeAll(where: { $0 == request})
145-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
146-
if inBackground {
147-
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
154+
self.dispatchQueue.async {
155+
self.updateRequestQueue.removeAll(where: { $0 == request})
156+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
157+
if inBackground {
158+
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
159+
}
148160
}
149161
} onFailure: { error in
150162
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor update properties request failed with error: \(error.debugDescription)")
151-
if let nsError = error as? NSError {
152-
let responseType = OSNetworkingUtils.getResponseStatusType(nsError.code)
153-
if responseType == .missing {
154-
// remove from cache and queue
155-
self.updateRequestQueue.removeAll(where: { $0 == request})
156-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
157-
// Logout if the user in the SDK is the same
158-
guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel)
159-
else {
160-
if inBackground {
161-
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
163+
self.dispatchQueue.async {
164+
if let nsError = error as? NSError {
165+
let responseType = OSNetworkingUtils.getResponseStatusType(nsError.code)
166+
if responseType == .missing {
167+
// remove from cache and queue
168+
self.updateRequestQueue.removeAll(where: { $0 == request})
169+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
170+
// Logout if the user in the SDK is the same
171+
guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel)
172+
else {
173+
if inBackground {
174+
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
175+
}
176+
return
162177
}
163-
return
178+
// The subscription has been deleted along with the user, so remove the subscription_id but keep the same push subscription model
179+
OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId = nil
180+
OneSignalUserManagerImpl.sharedInstance._logout()
181+
} else if responseType != .retryable {
182+
// Fail, no retry, remove from cache and queue
183+
self.updateRequestQueue.removeAll(where: { $0 == request})
184+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
164185
}
165-
// The subscription has been deleted along with the user, so remove the subscription_id but keep the same push subscription model
166-
OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId = nil
167-
OneSignalUserManagerImpl.sharedInstance._logout()
168-
} else if responseType != .retryable {
169-
// Fail, no retry, remove from cache and queue
170-
self.updateRequestQueue.removeAll(where: { $0 == request})
171-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
172186
}
173-
}
174-
if inBackground {
175-
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
187+
if inBackground {
188+
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
189+
}
176190
}
177191
}
178192
}
@@ -201,8 +215,10 @@ extension OSPropertyOperationExecutor {
201215
}
202216
}
203217
} else {
204-
updateRequestQueue.append(request)
205-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
218+
self.dispatchQueue.async {
219+
self.updateRequestQueue.append(request)
220+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
221+
}
206222
}
207223
}
208224
}

0 commit comments

Comments
 (0)