From 6c585a3cc887b84c78c2e1fc82e1868d8e97d65b Mon Sep 17 00:00:00 2001 From: Ricky <42879920+Rspoon3@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:48:17 -0500 Subject: [PATCH 1/3] Created ThreadSafeOrderedDictionary wrapper --- swift-sdk.xcodeproj/project.pbxproj | 8 ++ .../Internal/InAppManager+Functions.swift | 16 ++-- swift-sdk/Internal/InAppManager.swift | 8 +- .../ThreadSafeOrderedDictionary.swift | 85 +++++++++++++++++++ .../InAppMessageProcessorTests.swift | 4 +- .../ThreadSafeOrderedDictionaryTests.swift | 55 ++++++++++++ 6 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 swift-sdk/Internal/ThreadSafeOrderedDictionary.swift create mode 100644 tests/unit-tests/ThreadSafeOrderedDictionaryTests.swift diff --git a/swift-sdk.xcodeproj/project.pbxproj b/swift-sdk.xcodeproj/project.pbxproj index 992acd1e6..e671c82a0 100644 --- a/swift-sdk.xcodeproj/project.pbxproj +++ b/swift-sdk.xcodeproj/project.pbxproj @@ -185,6 +185,8 @@ 5B5AA717284F1A6D0093FED4 /* MockNetworkSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B5AA710284F1A6D0093FED4 /* MockNetworkSession.swift */; }; 5B6C3C1127CE871F00B9A753 /* NavInboxSessionUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B6C3C1027CE871F00B9A753 /* NavInboxSessionUITests.swift */; }; 5B88BC482805D09D004016E5 /* NetworkSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B88BC472805D09D004016E5 /* NetworkSession.swift */; }; + 94D8F9D42CDC291000D4CF53 /* ThreadSafeOrderedDictionary.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94D8F9D32CDC291000D4CF53 /* ThreadSafeOrderedDictionary.swift */; }; + 94D8F9D62CDC294300D4CF53 /* ThreadSafeOrderedDictionaryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94D8F9D52CDC294300D4CF53 /* ThreadSafeOrderedDictionaryTests.swift */; }; 9F76FFFF2B17884900962526 /* EmbeddedHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F76FFFE2B17884900962526 /* EmbeddedHelper.swift */; }; 9FF05EAC2AFEA5FA005311F7 /* MockAuthManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FF05EAB2AFEA5FA005311F7 /* MockAuthManager.swift */; }; 9FF05EAD2AFEA5FA005311F7 /* MockAuthManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FF05EAB2AFEA5FA005311F7 /* MockAuthManager.swift */; }; @@ -609,6 +611,8 @@ 5B6C3C1027CE871F00B9A753 /* NavInboxSessionUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavInboxSessionUITests.swift; sourceTree = ""; }; 5B88BC472805D09D004016E5 /* NetworkSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkSession.swift; sourceTree = ""; }; 5BFC7CED27FC9AF300E77479 /* inbox-ui-tests-app.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = "inbox-ui-tests-app.entitlements"; sourceTree = ""; }; + 94D8F9D32CDC291000D4CF53 /* ThreadSafeOrderedDictionary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThreadSafeOrderedDictionary.swift; sourceTree = ""; }; + 94D8F9D52CDC294300D4CF53 /* ThreadSafeOrderedDictionaryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThreadSafeOrderedDictionaryTests.swift; sourceTree = ""; }; 9F76FFFE2B17884900962526 /* EmbeddedHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmbeddedHelper.swift; sourceTree = ""; }; 9FF05EAB2AFEA5FA005311F7 /* MockAuthManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockAuthManager.swift; sourceTree = ""; }; AC02480722791E2100495FB9 /* IterableInboxNavigationViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IterableInboxNavigationViewController.swift; sourceTree = ""; }; @@ -1153,6 +1157,7 @@ 5536781E2576FF9000DB3652 /* IterableUtilTests.swift */, ACED4C00213F50B30055A497 /* LoggingTests.swift */, 55B37FC5229752DD0042F13A /* OrderedDictionaryTests.swift */, + 94D8F9D52CDC294300D4CF53 /* ThreadSafeOrderedDictionaryTests.swift */, ACEDF41E2183C436000B9BFE /* PendingTests.swift */, ); name = "foundational-tests"; @@ -1280,6 +1285,7 @@ AC776DA5211A1B8A00C27C27 /* IterableRequestUtil.swift */, AC72A0AD20CF4C16004D7997 /* IterableUtil.swift */, AC32E16721DD55B900BD4F83 /* OrderedDictionary.swift */, + 94D8F9D32CDC291000D4CF53 /* ThreadSafeOrderedDictionary.swift */, ACD8BF852757FC4C00C2EAB2 /* UIColor+Extension.swift */, ); name = Util; @@ -2112,6 +2118,7 @@ ACD2B83D25B0A74A005D7A90 /* Models.swift in Sources */, 55DD2027269E5EA300773CC7 /* InboxViewControllerViewModelView.swift in Sources */, AC1BED9523F1D4C700FDD75F /* MiscInboxClasses.swift in Sources */, + 94D8F9D42CDC291000D4CF53 /* ThreadSafeOrderedDictionary.swift in Sources */, 553449A129C2621E002E4599 /* EmbeddedMessagingProcessor.swift in Sources */, 556FB1EA244FAF6A00EDF6BD /* InAppPresenter.swift in Sources */, AC2AED4424EBC905000EE5F3 /* IterableTaskScheduler.swift in Sources */, @@ -2222,6 +2229,7 @@ AC8F35A2239806B500302994 /* InboxViewControllerViewModelTests.swift in Sources */, AC995F9A2166EEB50099A184 /* CommonMocks.swift in Sources */, 5588DFE128C046B7000697D7 /* MockLocalStorage.swift in Sources */, + 94D8F9D62CDC294300D4CF53 /* ThreadSafeOrderedDictionaryTests.swift in Sources */, 1CBFFE1B2A97AEEF00ED57EE /* EmbeddedMessagingProcessorTests.swift in Sources */, 5588DF8128C04494000697D7 /* MockUrlDelegate.swift in Sources */, 5585DF8F22A73390000A32B9 /* IterableInboxViewControllerTests.swift in Sources */, diff --git a/swift-sdk/Internal/InAppManager+Functions.swift b/swift-sdk/Internal/InAppManager+Functions.swift index d679e67ec..8c9340f47 100644 --- a/swift-sdk/Internal/InAppManager+Functions.swift +++ b/swift-sdk/Internal/InAppManager+Functions.swift @@ -5,14 +5,14 @@ import Foundation enum MessagesProcessorResult { - case show(message: IterableInAppMessage, messagesMap: OrderedDictionary) - case noShow(messagesMap: OrderedDictionary) + case show(message: IterableInAppMessage, messagesMap: ThreadSafeOrderedDictionary) + case noShow(messagesMap: ThreadSafeOrderedDictionary) } struct MessagesProcessor { init(inAppDelegate: IterableInAppDelegate, inAppDisplayChecker: InAppDisplayChecker, - messagesMap: OrderedDictionary) { + messagesMap: ThreadSafeOrderedDictionary) { ITBInfo() self.inAppDelegate = inAppDelegate @@ -97,18 +97,18 @@ struct MessagesProcessor { private let inAppDelegate: IterableInAppDelegate private let inAppDisplayChecker: InAppDisplayChecker - private var messagesMap: OrderedDictionary + private var messagesMap: ThreadSafeOrderedDictionary } struct MergeMessagesResult { let inboxChanged: Bool - let messagesMap: OrderedDictionary + let messagesMap: ThreadSafeOrderedDictionary let deliveredMessages: [IterableInAppMessage] } /// Merges the results and determines whether inbox changed needs to be fired. struct MessagesObtainedHandler { - init(messagesMap: OrderedDictionary, messages: [IterableInAppMessage]) { + init(messagesMap: ThreadSafeOrderedDictionary, messages: [IterableInAppMessage]) { ITBInfo() self.messagesMap = messagesMap self.messages = messages @@ -123,7 +123,7 @@ struct MessagesObtainedHandler { let addedInboxCount = addedMessages.reduce(0) { $1.saveToInbox ? $0 + 1 : $0 } var messagesOverwritten = 0 - var newMessagesMap = OrderedDictionary() + var newMessagesMap = ThreadSafeOrderedDictionary() messages.forEach { serverMessage in let messageId = serverMessage.messageId if let existingMessage = messagesMap[messageId] { @@ -145,7 +145,7 @@ struct MessagesObtainedHandler { deliveredMessages: deliveredMessages) } - private let messagesMap: OrderedDictionary + private let messagesMap: ThreadSafeOrderedDictionary private let messages: [IterableInAppMessage] // We should only overwrite if the server is read and client is not read. diff --git a/swift-sdk/Internal/InAppManager.swift b/swift-sdk/Internal/InAppManager.swift index eff886a5c..5da54332e 100644 --- a/swift-sdk/Internal/InAppManager.swift +++ b/swift-sdk/Internal/InAppManager.swift @@ -284,7 +284,7 @@ class InAppManager: NSObject, IterableInternalInAppManagerProtocol { lastSyncTime = dateProvider.currentDate } - private func getMessagesMap(fromMessagesProcessorResult messagesProcessorResult: MessagesProcessorResult) -> OrderedDictionary { + private func getMessagesMap(fromMessagesProcessorResult messagesProcessorResult: MessagesProcessorResult) -> ThreadSafeOrderedDictionary { switch messagesProcessorResult { case let .noShow(messagesMap: messagesMap): return messagesMap @@ -303,7 +303,7 @@ class InAppManager: NSObject, IterableInternalInAppManagerProtocol { } } - private func processAndShowMessage(messagesMap: OrderedDictionary) { + private func processAndShowMessage(messagesMap: ThreadSafeOrderedDictionary) { var processor = MessagesProcessor(inAppDelegate: inAppDelegate, inAppDisplayChecker: self, messagesMap: messagesMap) let messagesProcessorResult = processor.processMessages() self.messagesMap = getMessagesMap(fromMessagesProcessorResult: messagesProcessorResult) @@ -552,7 +552,7 @@ class InAppManager: NSObject, IterableInternalInAppManagerProtocol { private let notificationCenter: NotificationCenterProtocol private let persister: InAppPersistenceProtocol - private var messagesMap = OrderedDictionary() + private var messagesMap = ThreadSafeOrderedDictionary() private let dateProvider: DateProviderProtocol private var lastDismissedTime: Date? private var lastDisplayTime: Date? @@ -607,7 +607,7 @@ extension InAppManager: InAppNotifiable { ITBInfo() updateQueue.async { [weak self] in - if let _ = self?.messagesMap.filter({ $0.key == messageId }).first { + if let _ = self?.messagesMap.keys.filter({ $0 == messageId }).first { if let messagesMap = self?.messagesMap { self?.messagesMap.removeValue(forKey: messageId) self?.persister.persist(messagesMap.values) diff --git a/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift b/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift new file mode 100644 index 000000000..30cc63c81 --- /dev/null +++ b/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift @@ -0,0 +1,85 @@ +// +// ThreadSafeOrderedDictionary.swift +// swift-sdk +// +// Created by Ricky on 11/6/24. +// Copyright © 2024 Iterable. All rights reserved. +// + +import Foundation + +public final class ThreadSafeOrderedDictionary { + private var orderedDictionary = OrderedDictionary() + private let lock = NSLock() + + public var keys: [K] { + lock.withLock { + orderedDictionary.keys + } + } + + public var count: Int { + lock.withLock { + orderedDictionary.count + } + } + + public var values: [V] { + lock.withLock { + orderedDictionary.values + } + } + + public subscript(key: K) -> V? { + get { + lock.withLock { + orderedDictionary[key] + } + } + set { + lock.withLock { + orderedDictionary[key] = newValue + } + } + } + + @discardableResult public func updateValue(_ value: V?, forKey key: K) -> V? { + lock.withLock { + orderedDictionary.updateValue(value, forKey: key) + } + } + + @discardableResult public func removeValue(forKey key: K) -> V? { + lock.withLock { + orderedDictionary.removeValue(forKey: key) + } + } + + public func reset() { + lock.withLock { + orderedDictionary.reset() + } + } + + public func makeIterator() -> AnyIterator<(key: K, value: V)> { + lock.withLock { + orderedDictionary.makeIterator() + } + } + + public var description: String { + lock.withLock { + orderedDictionary.description + } + } +} + +// Conformance to ExpressibleByDictionaryLiteral +extension ThreadSafeOrderedDictionary: ExpressibleByDictionaryLiteral { + public convenience init(dictionaryLiteral elements: (K, V)...) { + self.init() + for (key, value) in elements { + self[key] = value + } + } +} diff --git a/tests/unit-tests/InAppMessageProcessorTests.swift b/tests/unit-tests/InAppMessageProcessorTests.swift index fbac26bfd..1cfdd20de 100644 --- a/tests/unit-tests/InAppMessageProcessorTests.swift +++ b/tests/unit-tests/InAppMessageProcessorTests.swift @@ -15,7 +15,7 @@ class InAppMessageProcessorTests: XCTestCase { let serverMessage = Self.makeEmptyInboxMessage(messageId) serverMessage.read = true - let messagesMap: OrderedDictionary = [messageId: localMessage] + let messagesMap: ThreadSafeOrderedDictionary = [messageId: localMessage] let newMessages = [serverMessage] let result = MessagesObtainedHandler(messagesMap: messagesMap, @@ -31,7 +31,7 @@ class InAppMessageProcessorTests: XCTestCase { let serverMessage2 = Self.makeEmptyInboxMessage("msg-3") serverMessage2.read = false - let messagesMap: OrderedDictionary = ["msg-1": localMessage] + let messagesMap: ThreadSafeOrderedDictionary = ["msg-1": localMessage] let newMessages = [serverMessage1, serverMessage2] let result = MessagesObtainedHandler(messagesMap: messagesMap, diff --git a/tests/unit-tests/ThreadSafeOrderedDictionaryTests.swift b/tests/unit-tests/ThreadSafeOrderedDictionaryTests.swift new file mode 100644 index 000000000..ddfbaf6ff --- /dev/null +++ b/tests/unit-tests/ThreadSafeOrderedDictionaryTests.swift @@ -0,0 +1,55 @@ +// +// ThreadSafeOrderedDictionaryTests.swift +// unit-tests +// +// Created by Ricky on 11/6/24. +// Copyright © 2024 Iterable. All rights reserved. +// + +import XCTest + +@testable import IterableSDK + +class ThreadSafeOrderedDictionaryTests: XCTestCase { + func testConcurrentAccess() { + let orderedDict = ThreadSafeOrderedDictionary() + + // Initialize with some data + orderedDict["initialKey"] = 0 + + let iterations = 1_000 + let concurrentQueue = DispatchQueue.global(qos: .userInitiated) + + // Dispatch group to wait for all tasks to complete + let dispatchGroup = DispatchGroup() + + // Concurrently write to the dictionary + dispatchGroup.enter() + concurrentQueue.async { + DispatchQueue.concurrentPerform(iterations: iterations) { i in + print(i, "Here 1") + orderedDict["key_\(i)"] = i + } + dispatchGroup.leave() + } + + // Concurrently read from the dictionary + dispatchGroup.enter() + concurrentQueue.async { + DispatchQueue.concurrentPerform(iterations: iterations) { i in + print(i, "Here 2") + _ = orderedDict["key_\(i % 10)"] + } + dispatchGroup.leave() + } + + // Wait for all operations to finish + let result = dispatchGroup.wait(timeout: .now() + 10) + + // Assert that all operations completed + XCTAssertEqual(result, .success, "Test timed out - possible deadlock or crash occurred.") + + // Check if dictionary contains at least one expected entry + XCTAssertGreaterThanOrEqual(orderedDict.count, 1, "Count should be greater than or equal to 1 after concurrent writes") + } +} From f055bb5662558eb37997c1be3e3af049f37ccdcf Mon Sep 17 00:00:00 2001 From: Sumeru Chatterjee Date: Mon, 2 Dec 2024 13:42:52 +0000 Subject: [PATCH 2/3] [MOB-10319] Change lock to NSRecursiveLock to prevent deadlock on the same thread --- swift-sdk/Internal/ThreadSafeOrderedDictionary.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift b/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift index 30cc63c81..e9726dcc0 100644 --- a/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift +++ b/swift-sdk/Internal/ThreadSafeOrderedDictionary.swift @@ -10,7 +10,7 @@ import Foundation public final class ThreadSafeOrderedDictionary { private var orderedDictionary = OrderedDictionary() - private let lock = NSLock() + private let lock = NSRecursiveLock() public var keys: [K] { lock.withLock { From 570c4b307a3454d9f5b2d03a20b75b1074672db9 Mon Sep 17 00:00:00 2001 From: Joao Dordio Date: Mon, 2 Dec 2024 14:45:11 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9A=A0=EF=B8=8F=20Fixing=20linter=20issu?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- swift-sdk/Internal/InAppManager+Functions.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift-sdk/Internal/InAppManager+Functions.swift b/swift-sdk/Internal/InAppManager+Functions.swift index 8c9340f47..6589ba10d 100644 --- a/swift-sdk/Internal/InAppManager+Functions.swift +++ b/swift-sdk/Internal/InAppManager+Functions.swift @@ -123,7 +123,7 @@ struct MessagesObtainedHandler { let addedInboxCount = addedMessages.reduce(0) { $1.saveToInbox ? $0 + 1 : $0 } var messagesOverwritten = 0 - var newMessagesMap = ThreadSafeOrderedDictionary() + let newMessagesMap = ThreadSafeOrderedDictionary() messages.forEach { serverMessage in let messageId = serverMessage.messageId if let existingMessage = messagesMap[messageId] {