diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index 434711331..6cac7ee9d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -53,6 +53,7 @@ 03E56DD328405F4A006AA1DA /* OneSignalAppDelegateOverrider.m in Sources */ = {isa = PBXBuildFile; fileRef = 03E56DD228405F4A006AA1DA /* OneSignalAppDelegateOverrider.m */; }; 16664C4C25DDB195003B8A14 /* NSTimeZoneOverrider.m in Sources */ = {isa = PBXBuildFile; fileRef = 16664C4B25DDB195003B8A14 /* NSTimeZoneOverrider.m */; }; 37E6B2BB19D9CAF300D0C601 /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 37E6B2BA19D9CAF300D0C601 /* UIKit.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; + 3C05904B2B86BC9E00450D48 /* UnfairLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3C05904A2B86BC9E00450D48 /* UnfairLock.swift */; }; 3C0EF49E28A1DBCB00E5434B /* OSUserInternalImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3C0EF49D28A1DBCB00E5434B /* OSUserInternalImpl.swift */; }; 3C115165289A259500565C41 /* OneSignalOSCore.docc in Sources */ = {isa = PBXBuildFile; fileRef = 3C115164289A259500565C41 /* OneSignalOSCore.docc */; }; 3C115171289A259500565C41 /* OneSignalOSCore.h in Headers */ = {isa = PBXBuildFile; fileRef = 3C115163289A259500565C41 /* OneSignalOSCore.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -943,6 +944,7 @@ 1AF75EAD1E8567FD0097B315 /* NSString+OneSignal.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSString+OneSignal.m"; sourceTree = ""; }; 37747F9319147D6500558FAD /* libOneSignal.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libOneSignal.a; sourceTree = BUILT_PRODUCTS_DIR; }; 37E6B2BA19D9CAF300D0C601 /* UIKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = UIKit.framework; path = System/Library/Frameworks/UIKit.framework; sourceTree = SDKROOT; }; + 3C05904A2B86BC9E00450D48 /* UnfairLock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnfairLock.swift; sourceTree = ""; }; 3C0EF49D28A1DBCB00E5434B /* OSUserInternalImpl.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OSUserInternalImpl.swift; sourceTree = ""; }; 3C115161289A259500565C41 /* OneSignalOSCore.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OneSignalOSCore.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 3C115163289A259500565C41 /* OneSignalOSCore.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = OneSignalOSCore.h; sourceTree = ""; }; @@ -1638,6 +1640,14 @@ name = Frameworks; sourceTree = ""; }; + 3C0590492B86BC5300450D48 /* Utils */ = { + isa = PBXGroup; + children = ( + 3C05904A2B86BC9E00450D48 /* UnfairLock.swift */, + ); + path = Utils; + sourceTree = ""; + }; 3C115162289A259500565C41 /* OneSignalOSCore */ = { isa = PBXGroup; children = ( @@ -1651,6 +1661,7 @@ isa = PBXGroup; children = ( 3C115163289A259500565C41 /* OneSignalOSCore.h */, + 3C0590492B86BC5300450D48 /* Utils */, 3C115188289ADEA300565C41 /* OSModelStore.swift */, 3C115186289ADE7700565C41 /* OSModelStoreListener.swift */, 3C115184289ADE4F00565C41 /* OSModel.swift */, @@ -3327,6 +3338,7 @@ 3C115187289ADE7700565C41 /* OSModelStoreListener.swift in Sources */, 3CE5F9E3289D88DC004A156E /* OSModelStoreChangedHandler.swift in Sources */, 3C2D8A5928B4C4E300BE41F6 /* OSDelta.swift in Sources */, + 3C05904B2B86BC9E00450D48 /* UnfairLock.swift in Sources */, 3C11518D289AF5E800565C41 /* OSModelChangedHandler.swift in Sources */, 3C8E6DF928A6D89E0031E48A /* OSOperationExecutor.swift in Sources */, ); diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationRepo.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationRepo.swift index d13d38162..6da7b4553 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationRepo.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationRepo.swift @@ -36,10 +36,13 @@ public class OSOperationRepo: NSObject { public static let sharedInstance = OSOperationRepo() private var hasCalledStart = false + // The Operation Repo dispatch queue, serial. This synchronizes access to `deltaQueue` and flushing behavior. + private let dispatchQueue = DispatchQueue(label: "OneSignal.OSOperationRepo", target: .global()) + // Maps delta names to the interfaces for the operation executors var deltasToExecutorMap: [String: OSOperationExecutor] = [:] var executors: [OSOperationExecutor] = [] - var deltaQueue: [OSDelta] = [] + private var deltaQueue: [OSDelta] = [] // TODO: This could come from a config, plist, method, remote params var pollIntervalMilliseconds = Int(POLL_INTERVAL_MS) @@ -62,7 +65,7 @@ public class OSOperationRepo: NSObject { OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo calling start()") // register as user observer NotificationCenter.default.addObserver(self, - selector: #selector(self.flushDeltaQueue), + selector: #selector(self.addFlushDeltaQueueToDispatchQueue), name: Notification.Name(OS_ON_USER_WILL_CHANGE), object: nil) // Read the Deltas from cache, if any... @@ -76,7 +79,7 @@ public class OSOperationRepo: NSObject { } private func pollFlushQueue() { - DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(pollIntervalMilliseconds)) { [weak self] in + self.dispatchQueue.asyncAfter(deadline: .now() + .milliseconds(pollIntervalMilliseconds)) { [weak self] in self?.flushDeltaQueue() self?.pollFlushQueue() } @@ -101,14 +104,21 @@ public class OSOperationRepo: NSObject { return } start() - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo enqueueDelta: \(delta)") - deltaQueue.append(delta) + self.dispatchQueue.async { + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo enqueueDelta: \(delta)") + self.deltaQueue.append(delta) + // Persist the deltas (including new delta) to storage + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_OPERATION_REPO_DELTA_QUEUE_KEY, withValue: self.deltaQueue) + } + } - // Persist the deltas (including new delta) to storage - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_OPERATION_REPO_DELTA_QUEUE_KEY, withValue: self.deltaQueue) + @objc public func addFlushDeltaQueueToDispatchQueue(inBackground: Bool = false) { + self.dispatchQueue.async { + self.flushDeltaQueue(inBackground: inBackground) + } } - @objc public func flushDeltaQueue(inBackground: Bool = false) { + private func flushDeltaQueue(inBackground: Bool = false) { guard !paused else { OneSignalLog.onesignalLog(.LL_DEBUG, message: "OSOperationRepo not flushing queue due to being paused") return @@ -122,16 +132,17 @@ public class OSOperationRepo: NSObject { OSBackgroundTaskManager.beginBackgroundTask(OPERATION_REPO_BACKGROUND_TASK) } - start() - if !deltaQueue.isEmpty { - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo flushDeltaQueue in background: \(inBackground) with queue: \(deltaQueue)") + self.start() + + if !self.deltaQueue.isEmpty { + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo flushDeltaQueue in background: \(inBackground) with queue: \(self.deltaQueue)") } var index = 0 - for delta in deltaQueue { - if let executor = deltasToExecutorMap[delta.name] { + for delta in self.deltaQueue { + if let executor = self.deltasToExecutorMap[delta.name] { executor.enqueueDelta(delta) - deltaQueue.remove(at: index) + self.deltaQueue.remove(at: index) } else { // keep in queue if no executor matches, we may not have the executor available yet index += 1 @@ -141,17 +152,16 @@ public class OSOperationRepo: NSObject { // Persist the deltas (including removed deltas) to storage after they are divvy'd up to executors. OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_OPERATION_REPO_DELTA_QUEUE_KEY, withValue: self.deltaQueue) - for executor in executors { + for executor in self.executors { executor.cacheDeltaQueue() } - for executor in executors { + for executor in self.executors { executor.processDeltaQueue(inBackground: inBackground) } if inBackground { OSBackgroundTaskManager.endBackgroundTask(OPERATION_REPO_BACKGROUND_TASK) } - } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Utils/UnfairLock.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Utils/UnfairLock.swift new file mode 100644 index 000000000..9d9f69ad2 --- /dev/null +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Utils/UnfairLock.swift @@ -0,0 +1,47 @@ +/* + Modified MIT License + + Copyright 2024 OneSignal + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + 1. The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + 2. All copies of substantial portions of the Software may only be used in connection + with services provided by OneSignal. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + */ + +// Taken from https://swiftrocks.com/thread-safety-in-swift +// Read http://www.russbishop.net/the-law for more information on why this is necessary +public final class UnfairLock { + private var _lock: UnsafeMutablePointer + + public init() { + _lock = UnsafeMutablePointer.allocate(capacity: 1) + _lock.initialize(to: os_unfair_lock()) + } + + deinit { + _lock.deallocate() + } + + public func locked(_ closure: () throws -> ReturnValue) rethrows -> ReturnValue { + os_unfair_lock_lock(_lock) + defer { os_unfair_lock_unlock(_lock) } + return try closure() + } +} diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift index f01f60b89..7474284a7 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift @@ -33,6 +33,9 @@ class OSPropertyOperationExecutor: OSOperationExecutor { var deltaQueue: [OSDelta] = [] var updateRequestQueue: [OSRequestUpdateProperties] = [] + // The property executor dispatch queue, serial. This synchronizes access to `deltaQueue` and `updateRequestQueue`. + private let dispatchQueue = DispatchQueue(label: "OneSignal.OSPropertyOperationExecutor", target: .global()) + init() { // Read unfinished deltas from cache, if any... // Note that we should only have deltas for the current user as old ones are flushed.. @@ -78,42 +81,49 @@ class OSPropertyOperationExecutor: OSOperationExecutor { } func enqueueDelta(_ delta: OSDelta) { - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)") - deltaQueue.append(delta) + self.dispatchQueue.async { + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)") + self.deltaQueue.append(delta) + } } func cacheDeltaQueue() { - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) + self.dispatchQueue.async { + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) + } } func processDeltaQueue(inBackground: Bool) { - if !deltaQueue.isEmpty { - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(deltaQueue)") - } - for delta in deltaQueue { - guard let model = delta.model as? OSPropertiesModel else { - // Log error - continue + self.dispatchQueue.async { + if !self.deltaQueue.isEmpty { + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)") } + for delta in self.deltaQueue { + guard let model = delta.model as? OSPropertiesModel else { + // Log error + continue + } - let request = OSRequestUpdateProperties( - properties: [delta.property: delta.value], - deltas: nil, - refreshDeviceMetadata: false, // Sort this out. - modelToUpdate: model, - identityModel: OneSignalUserManagerImpl.sharedInstance.user.identityModel // TODO: Make sure this is ok - ) - updateRequestQueue.append(request) - } - self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue + let request = OSRequestUpdateProperties( + properties: [delta.property: delta.value], + deltas: nil, + refreshDeviceMetadata: false, // Sort this out. + modelToUpdate: model, + identityModel: OneSignalUserManagerImpl.sharedInstance.user.identityModel // TODO: Make sure this is ok + ) + self.updateRequestQueue.append(request) + } + self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue - // persist executor's requests (including new request) to storage - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) + // persist executor's requests (including new request) to storage + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) // This should be empty, can remove instead? - processRequestQueue(inBackground: inBackground) + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) // This should be empty, can remove instead? + self.processRequestQueue(inBackground: inBackground) + } } + // This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue. func processRequestQueue(inBackground: Bool) { if updateRequestQueue.isEmpty { return @@ -141,38 +151,42 @@ class OSPropertyOperationExecutor: OSOperationExecutor { OneSignalCore.sharedClient().execute(request) { _ in // On success, remove request from cache, and we do need to hydrate // TODO: We need to hydrate after all ? What why ? - self.updateRequestQueue.removeAll(where: { $0 == request}) - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) - if inBackground { - OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + self.dispatchQueue.async { + self.updateRequestQueue.removeAll(where: { $0 == request}) + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) + if inBackground { + OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + } } } onFailure: { error in OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor update properties request failed with error: \(error.debugDescription)") - if let nsError = error as? NSError { - let responseType = OSNetworkingUtils.getResponseStatusType(nsError.code) - if responseType == .missing { - // remove from cache and queue - self.updateRequestQueue.removeAll(where: { $0 == request}) - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) - // Logout if the user in the SDK is the same - guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) - else { - if inBackground { - OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + self.dispatchQueue.async { + if let nsError = error as? NSError { + let responseType = OSNetworkingUtils.getResponseStatusType(nsError.code) + if responseType == .missing { + // remove from cache and queue + self.updateRequestQueue.removeAll(where: { $0 == request}) + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) + // Logout if the user in the SDK is the same + guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) + else { + if inBackground { + OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + } + return } - return + // The subscription has been deleted along with the user, so remove the subscription_id but keep the same push subscription model + OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId = nil + OneSignalUserManagerImpl.sharedInstance._logout() + } else if responseType != .retryable { + // Fail, no retry, remove from cache and queue + self.updateRequestQueue.removeAll(where: { $0 == request}) + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) } - // The subscription has been deleted along with the user, so remove the subscription_id but keep the same push subscription model - OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId = nil - OneSignalUserManagerImpl.sharedInstance._logout() - } else if responseType != .retryable { - // Fail, no retry, remove from cache and queue - self.updateRequestQueue.removeAll(where: { $0 == request}) - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) } - } - if inBackground { - OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + if inBackground { + OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier) + } } } } @@ -201,8 +215,10 @@ extension OSPropertyOperationExecutor { } } } else { - updateRequestQueue.append(request) - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) + self.dispatchQueue.async { + self.updateRequestQueue.append(request) + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) + } } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift index b915030cc..72a5671e2 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift @@ -31,14 +31,19 @@ import OneSignalOSCore class OSIdentityModel: OSModel { var onesignalId: String? { - return aliases[OS_ONESIGNAL_ID] + aliasesLock.locked { + return aliases[OS_ONESIGNAL_ID] + } } var externalId: String? { - return aliases[OS_EXTERNAL_ID] + aliasesLock.locked { + return aliases[OS_EXTERNAL_ID] + } } var aliases: [String: String] = [:] + private let aliasesLock = UnfairLock() // TODO: We need to make this token secure public var jwtBearerToken: String? @@ -69,22 +74,28 @@ class OSIdentityModel: OSModel { Called to clear the model's data in preparation for hydration via a fetch user call. */ func clearData() { - self.aliases = [:] + aliasesLock.locked { + self.aliases = [:] + } } // MARK: - Alias Methods func addAliases(_ aliases: [String: String]) { - for (label, id) in aliases { - self.aliases[label] = id + aliasesLock.locked { + for (label, id) in aliases { + self.aliases[label] = id + } + self.set(property: "aliases", newValue: aliases) } - self.set(property: "aliases", newValue: aliases) } func removeAliases(_ labels: [String]) { - for label in labels { - self.aliases.removeValue(forKey: label) - self.set(property: "aliases", newValue: [label: ""]) + aliasesLock.locked { + for label in labels { + self.aliases.removeValue(forKey: label) + self.set(property: "aliases", newValue: [label: ""]) + } } } @@ -93,18 +104,20 @@ class OSIdentityModel: OSModel { var newOnesignalId: String? var newExternalId: String? - for property in response { - switch property.key { - case "external_id": - newExternalId = property.value as? String - aliases[OS_EXTERNAL_ID] = newExternalId - case "onesignal_id": - newOnesignalId = property.value as? String - aliases[OS_ONESIGNAL_ID] = newOnesignalId - default: - aliases[property.key] = property.value as? String + aliasesLock.locked { + for property in response { + switch property.key { + case "external_id": + newExternalId = property.value as? String + aliases[OS_EXTERNAL_ID] = newExternalId + case "onesignal_id": + newOnesignalId = property.value as? String + aliases[OS_ONESIGNAL_ID] = newOnesignalId + default: + aliases[property.key] = property.value as? String + } + self.set(property: "aliases", newValue: aliases) } - self.set(property: "aliases", newValue: aliases) } fireUserStateChanged(newOnesignalId: newOnesignalId, newExternalId: newExternalId) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModel.swift index eb9e59bc1..edc23fc88 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModel.swift @@ -82,6 +82,7 @@ class OSPropertiesModel: OSModel { var timezoneId = TimeZone.current.identifier var tags: [String: String] = [:] + private let tagsLock = UnfairLock() // MARK: - Initialization @@ -124,25 +125,31 @@ class OSPropertiesModel: OSModel { */ func clearData() { // TODO: What about language, lat, long? - self.tags = [:] + tagsLock.locked { + self.tags = [:] + } } // MARK: - Tag Methods func addTags(_ tags: [String: String]) { - for (key, value) in tags { - self.tags[key] = value + tagsLock.locked { + for (key, value) in tags { + self.tags[key] = value + } + self.set(property: "tags", newValue: tags) } - self.set(property: "tags", newValue: tags) } func removeTags(_ tags: [String]) { - var tagsToSend: [String: String] = [:] - for tag in tags { - self.tags.removeValue(forKey: tag) - tagsToSend[tag] = "" + tagsLock.locked { + var tagsToSend: [String: String] = [:] + for tag in tags { + self.tags.removeValue(forKey: tag) + tagsToSend[tag] = "" + } + self.set(property: "tags", newValue: tagsToSend) } - self.set(property: "tags", newValue: tagsToSend) } public override func hydrateModel(_ response: [String: Any]) { @@ -151,7 +158,9 @@ class OSPropertiesModel: OSModel { case "language": self.language = property.value as? String case "tags": - self.tags = property.value as? [String: String] ?? [:] + tagsLock.locked { + self.tags = property.value as? [String: String] ?? [:] + } default: OneSignalLog.onesignalLog(.LL_DEBUG, message: "Not hydrating properties model for property: \(property)") } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index 36d8ceaff..e63b4da46 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -594,7 +594,7 @@ extension OneSignalUserManagerImpl { */ @objc public func runBackgroundTasks() { - OSOperationRepo.sharedInstance.flushDeltaQueue(inBackground: true) + OSOperationRepo.sharedInstance.addFlushDeltaQueueToDispatchQueue(inBackground: true) } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift index a53197056..a65947d22 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift @@ -29,6 +29,7 @@ import XCTest import OneSignalCore import OneSignalCoreMocks import OneSignalUserMocks +import OneSignalOSCore @testable import OneSignalUser final class OneSignalUserTests: XCTestCase { @@ -62,4 +63,34 @@ final class OneSignalUserTests: XCTestCase { XCTAssertEqual(identityModelStoreExternalId, "my-external-id") XCTAssertEqual(userInstanceExternalId, "my-external-id") } + + /** + This test reproduces a crash in the Operation Repo's flushing delta queue. + It is possible for two threads to flush concurrently. + However, this test does not crash 100% of the time. + */ + func testOperationRepoFlushingConcurrency() throws { + /* Setup */ + OneSignalCore.setSharedClient(MockOneSignalClient()) + + /* When */ + + // 1. Enqueue 10 Deltas to the Operation Repo + for num in 0...9 { + OneSignalUserManagerImpl.sharedInstance.addTag(key: "tag\(num)", value: "value") + } + + // 2. Flush the delta queue from 4 multiple threads + for _ in 1...4 { + DispatchQueue.global().async { + print("🧪 flushDeltaQueue on thread \(Thread.current)") + OSOperationRepo.sharedInstance.addFlushDeltaQueueToDispatchQueue() + } + } + + /* Then */ + // There are two places that can crash, as multiple threads are manipulating arrays: + // 1. OpRepo: `deltaQueue.remove(at: index)` index out of bounds + // 2. OSPropertyOperationExecutor: `deltaQueue.append(delta)` EXC_BAD_ACCESS + } } diff --git a/iOS_SDK/OneSignalSDK/Source/OSBackgroundTaskHandlerImpl.m b/iOS_SDK/OneSignalSDK/Source/OSBackgroundTaskHandlerImpl.m index 79b2dac43..9db3fdcf3 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSBackgroundTaskHandlerImpl.m +++ b/iOS_SDK/OneSignalSDK/Source/OSBackgroundTaskHandlerImpl.m @@ -50,21 +50,30 @@ - (void)beginBackgroundTask:(NSString * _Nonnull)taskIdentifier { @"OSBackgroundTaskManagerImpl: expirationHandler called for %@", taskIdentifier]]; [self endBackgroundTask:taskIdentifier]; }]; - tasks[taskIdentifier] = [NSNumber numberWithUnsignedLong:uiIdentifier]; + @synchronized (tasks) { + tasks[taskIdentifier] = [NSNumber numberWithUnsignedLong:uiIdentifier]; + } } - (void)endBackgroundTask:(NSString * _Nonnull)taskIdentifier { - UIBackgroundTaskIdentifier uiIdentifier = [[tasks objectForKey:taskIdentifier] unsignedLongValue]; - [OneSignalLog onesignalLog:ONE_S_LL_DEBUG - message:[NSString stringWithFormat: - @"OSBackgroundTaskManagerImpl:endBackgroundTask: %@ with UIBackgroundTaskIdentifier %lu", - taskIdentifier, (unsigned long)uiIdentifier]]; - [UIApplication.sharedApplication endBackgroundTask:uiIdentifier]; + @synchronized (tasks) { + UIBackgroundTaskIdentifier uiIdentifier = [[tasks objectForKey:taskIdentifier] unsignedLongValue]; + [OneSignalLog onesignalLog:ONE_S_LL_DEBUG + message:[NSString stringWithFormat: + @"OSBackgroundTaskManagerImpl:endBackgroundTask: %@ with UIBackgroundTaskIdentifier %lu", + taskIdentifier, (unsigned long)uiIdentifier]]; + [UIApplication.sharedApplication endBackgroundTask:uiIdentifier]; + } [self setTaskInvalid:taskIdentifier]; } +/** + This method is called when the background task ends or directly by other services within the SDK + */ - (void)setTaskInvalid:(NSString * _Nonnull)taskIdentifier { - tasks[taskIdentifier] = [NSNumber numberWithUnsignedLong:UIBackgroundTaskInvalid]; + @synchronized (tasks) { + tasks[taskIdentifier] = [NSNumber numberWithUnsignedLong:UIBackgroundTaskInvalid]; + } } @end