From e13febabcf305468804a52afb670b5986b79562d Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 10:48:15 +0100 Subject: [PATCH 01/15] fix: add sync queue --- Sources/UnleashProxyClientSwift/Poller.swift | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 74f9395..361c115 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -22,19 +22,26 @@ public protocol StorageProvider { public class DictionaryStorageProvider: StorageProvider { private var storage: [String: Toggle] = [:] + private let queue = DispatchQueue(label: "com.unleash.storageprovider", attributes: .concurrent) public init() {} public func set(value: Toggle?, key: String) { - storage[key] = value + queue.async(flags: .barrier) { + storage[key] = value + } } public func value(key: String) -> Toggle? { - return storage[key] + queue.sync { + return storage[key] + } } public func clear() { - storage = [:] + queue.sync { + storage = [:] + } } } From 9f45234c741764248fe7d5ba05c636c8eeb8c695 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 10:50:05 +0100 Subject: [PATCH 02/15] fix: add self --- Sources/UnleashProxyClientSwift/Poller.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 361c115..e95b5e7 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -28,19 +28,19 @@ public class DictionaryStorageProvider: StorageProvider { public func set(value: Toggle?, key: String) { queue.async(flags: .barrier) { - storage[key] = value + self.storage[key] = value } } public func value(key: String) -> Toggle? { queue.sync { - return storage[key] + return self.storage[key] } } public func clear() { queue.sync { - storage = [:] + self.storage = [:] } } } From b04690aaf3bf6e318554b9cb0ce28bb0bd9e05f2 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 10:52:23 +0100 Subject: [PATCH 03/15] fix: lock --- Sources/UnleashProxyClientSwift/Poller.swift | 21 ++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index e95b5e7..1105bab 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -22,26 +22,27 @@ public protocol StorageProvider { public class DictionaryStorageProvider: StorageProvider { private var storage: [String: Toggle] = [:] - private let queue = DispatchQueue(label: "com.unleash.storageprovider", attributes: .concurrent) + private let lock = NSLock() public init() {} public func set(value: Toggle?, key: String) { - queue.async(flags: .barrier) { - self.storage[key] = value - } + lock.lock() + storage[key] = value + lock.unlock() } public func value(key: String) -> Toggle? { - queue.sync { - return self.storage[key] - } + lock.lock() + let value = storage[key] + lock.unlock() + return value } public func clear() { - queue.sync { - self.storage = [:] - } + lock.lock() + storage = [:] + lock.unlock() } } From dd65301b646953ce0b5d10ae9a7a1f76b79c06e3 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 10:55:57 +0100 Subject: [PATCH 04/15] fix: tweak dispatchqueue --- Sources/UnleashProxyClientSwift/Poller.swift | 21 ++++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 1105bab..3a95d5a 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -22,27 +22,26 @@ public protocol StorageProvider { public class DictionaryStorageProvider: StorageProvider { private var storage: [String: Toggle] = [:] - private let lock = NSLock() + private let queue = DispatchQueue(label: "com.unleash.storageprovider") public init() {} public func set(value: Toggle?, key: String) { - lock.lock() - storage[key] = value - lock.unlock() + queue.async { + self.storage[key] = value + } } public func value(key: String) -> Toggle? { - lock.lock() - let value = storage[key] - lock.unlock() - return value + queue.sync { + return self.storage[key] + } } public func clear() { - lock.lock() - storage = [:] - lock.unlock() + queue.sync { + self.storage = [:] + } } } From 5bba12bfa5884d80b41af3c561a73479ace350b3 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:20:07 +0100 Subject: [PATCH 05/15] fix: github action --- .github/workflows/build.yml | 2 +- Sources/UnleashProxyClientSwift/Poller.swift | 12 ++++++------ .../UnleashClientIntegrationTest.swift | 7 +++++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 32e8c42..0314eda 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,4 +12,4 @@ jobs: - name: Build run: swift build - name: Run tests - run: swift test + run: swift test --sanitize=thread diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 3a95d5a..5694cbc 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -27,21 +27,21 @@ public class DictionaryStorageProvider: StorageProvider { public init() {} public func set(value: Toggle?, key: String) { - queue.async { + self.storage[key] = value - } + } public func value(key: String) -> Toggle? { - queue.sync { + return self.storage[key] - } + } public func clear() { - queue.sync { + self.storage = [:] - } + } } diff --git a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift index 73a6bae..cb49c72 100644 --- a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift +++ b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift @@ -45,6 +45,13 @@ class UnleashIntegrationTests: XCTestCase { }); unleashClient.start() + // Run isEnabled many times to trigger a data race when the poller is updating the data cache + // This is to test that the poller is thread safe, and you can verify this by running the test with + // swift test --sanitize=thread + for _ in 1...15000 { + let result = unleashClient.isEnabled(name: "dataRaceTest") + XCTAssertFalse(result) + } wait(for: [expectation], timeout: 5) } From 3aa5bb38e39ee04f82b33a17310d6ded884b0f25 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:22:13 +0100 Subject: [PATCH 06/15] fix: command --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0314eda..f120eba 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,4 +12,4 @@ jobs: - name: Build run: swift build - name: Run tests - run: swift test --sanitize=thread + run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" && exit 1' From c05ca437e4fa5900a7c4209cb1889d08fdbead85 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:24:46 +0100 Subject: [PATCH 07/15] feat: add queue to dictionary provider --- Sources/UnleashProxyClientSwift/Poller.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 5694cbc..3a95d5a 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -27,21 +27,21 @@ public class DictionaryStorageProvider: StorageProvider { public init() {} public func set(value: Toggle?, key: String) { - + queue.async { self.storage[key] = value - + } } public func value(key: String) -> Toggle? { - + queue.sync { return self.storage[key] - + } } public func clear() { - + queue.sync { self.storage = [:] - + } } } From fd0f5abda144743f791a877383e90edc4b10e620 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:29:44 +0100 Subject: [PATCH 08/15] refactor: github command --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f120eba..67e7bf2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,4 +12,4 @@ jobs: - name: Build run: swift build - name: Run tests - run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" && exit 1' + run: 'swift test --sanitize=thread 2>&1 | tee /dev/tty | grep "ThreadSanitizer: data race" && exit 1' From cf022aff4bcfca3ac0ae9d3f8f01f3751c768680 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:38:54 +0100 Subject: [PATCH 09/15] fix: break gh into jobs --- .github/workflows/build.yml | 12 +++++++++++- UnleashProxyClientSwift.podspec | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 67e7bf2..b4376e1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,5 +11,15 @@ jobs: - uses: actions/checkout@v2 - name: Build run: swift build + test: + runs-on: macos-latest + steps: + - uses: actions/checkout@v2 + - name: Run tests + run: swift test --sanitize=thread + test-sanitize: + runs-on: macos-latest + steps: + - uses: actions/checkout@v2 - name: Run tests - run: 'swift test --sanitize=thread 2>&1 | tee /dev/tty | grep "ThreadSanitizer: data race" && exit 1' + run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" && exit 1' diff --git a/UnleashProxyClientSwift.podspec b/UnleashProxyClientSwift.podspec index 684cf00..746ddfb 100644 --- a/UnleashProxyClientSwift.podspec +++ b/UnleashProxyClientSwift.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |spec| spec.name = "UnleashProxyClientSwift" -spec.version = "1.1.0" +spec.version = "1.1.1" spec.summary = "Allows frontend clients to talk to unleash through the unleash edge, frontend API or the (deprecated) unleash proxy" spec.homepage = "https://www.getunleash.io" spec.license = { :type => "MIT", :file => "LICENSE" } From 491c1f7928adfea6de994f16d43ad245363ab151 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:45:21 +0100 Subject: [PATCH 10/15] fix: gh command --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b4376e1..696127e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,4 +22,4 @@ jobs: steps: - uses: actions/checkout@v2 - name: Run tests - run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" && exit 1' + run: 'swift test --sanitize=thread 2>&1 | grep -q "ThreadSanitizer: data race" && exit 1' From b94a45333611bd2b8556c4a566f6b9bdde549c6b Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:47:53 +0100 Subject: [PATCH 11/15] fix: force grep to continue --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 696127e..b12e99f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,4 +22,4 @@ jobs: steps: - uses: actions/checkout@v2 - name: Run tests - run: 'swift test --sanitize=thread 2>&1 | grep -q "ThreadSanitizer: data race" && exit 1' + run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" || exit 0; exit 1' From 35f2d6a63769c6ea34ecb02c1b7205dc7a31ac93 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 11:49:35 +0100 Subject: [PATCH 12/15] fix: verify failing ci --- Sources/UnleashProxyClientSwift/Poller.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index 3a95d5a..d954362 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -27,21 +27,21 @@ public class DictionaryStorageProvider: StorageProvider { public init() {} public func set(value: Toggle?, key: String) { - queue.async { + self.storage[key] = value - } + } public func value(key: String) -> Toggle? { - queue.sync { + return self.storage[key] - } + } public func clear() { - queue.sync { + self.storage = [:] - } + } } From b2b899d1a6e4b499a1bb1c370e52085191ab3a5e Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 12:01:20 +0100 Subject: [PATCH 13/15] fix: increase iterations --- .../UnleashClientIntegrationTest.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift index cb49c72..fce4c45 100644 --- a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift +++ b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift @@ -48,7 +48,7 @@ class UnleashIntegrationTests: XCTestCase { // Run isEnabled many times to trigger a data race when the poller is updating the data cache // This is to test that the poller is thread safe, and you can verify this by running the test with // swift test --sanitize=thread - for _ in 1...15000 { + for _ in 1...25000 { let result = unleashClient.isEnabled(name: "dataRaceTest") XCTAssertFalse(result) } From ecf5821269110b506e7f3400b9ba18a2bf9b72e2 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 12:04:47 +0100 Subject: [PATCH 14/15] fix: update readme --- .github/workflows/build.yml | 8 +------- README.md | 10 ++++++++++ .../UnleashClientIntegrationTest.swift | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b12e99f..34f5d5a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,10 +16,4 @@ jobs: steps: - uses: actions/checkout@v2 - name: Run tests - run: swift test --sanitize=thread - test-sanitize: - runs-on: macos-latest - steps: - - uses: actions/checkout@v2 - - name: Run tests - run: 'swift test --sanitize=thread 2>&1 | grep "ThreadSanitizer: data race" || exit 0; exit 1' + run: swift test --sanitize=thread \ No newline at end of file diff --git a/README.md b/README.md index d3b3cf3..3d5a2c9 100644 --- a/README.md +++ b/README.md @@ -184,3 +184,13 @@ Once that succeeds, you can do the actual release: ```sh pod trunk push UnleashProxyClientSwift.podspec --allow-warnings ``` + +## Testing + +In order to test this package you can run the swift test command. To test thread safety, run swift test with: + +``` +swift test --sanitize=thread +``` + +This will give you warnings in the console when you have any data races. diff --git a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift index fce4c45..cb49c72 100644 --- a/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift +++ b/Tests/UnleashProxyClientSwiftTests/UnleashClientIntegrationTest.swift @@ -48,7 +48,7 @@ class UnleashIntegrationTests: XCTestCase { // Run isEnabled many times to trigger a data race when the poller is updating the data cache // This is to test that the poller is thread safe, and you can verify this by running the test with // swift test --sanitize=thread - for _ in 1...25000 { + for _ in 1...15000 { let result = unleashClient.isEnabled(name: "dataRaceTest") XCTAssertFalse(result) } From 18339964304014a2ea3479369b7ef4069acd916b Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Fri, 1 Dec 2023 12:05:33 +0100 Subject: [PATCH 15/15] fix: add queue back to dict --- Sources/UnleashProxyClientSwift/Poller.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/UnleashProxyClientSwift/Poller.swift b/Sources/UnleashProxyClientSwift/Poller.swift index d954362..3a95d5a 100644 --- a/Sources/UnleashProxyClientSwift/Poller.swift +++ b/Sources/UnleashProxyClientSwift/Poller.swift @@ -27,21 +27,21 @@ public class DictionaryStorageProvider: StorageProvider { public init() {} public func set(value: Toggle?, key: String) { - + queue.async { self.storage[key] = value - + } } public func value(key: String) -> Toggle? { - + queue.sync { return self.storage[key] - + } } public func clear() { - + queue.sync { self.storage = [:] - + } } }