From 183552d6d693dc657b7f746d39ad38bbbe34df84 Mon Sep 17 00:00:00 2001 From: Rolandas Razma Date: Mon, 20 Jan 2020 10:31:27 +0000 Subject: [PATCH 1/2] use NSLock() to prevent records: RecordSet modification from different queue at the same time --- Sources/Apollo/InMemoryNormalizedCache.swift | 7 ++ .../FetchQueryTests.swift | 91 +++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 3c19fce743..084c7b52f7 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -2,6 +2,7 @@ import Foundation public final class InMemoryNormalizedCache: NormalizedCache { private var records: RecordSet + private let recordsLock = NSLock() public init(records: RecordSet = RecordSet()) { self.records = records @@ -10,7 +11,9 @@ public final class InMemoryNormalizedCache: NormalizedCache { public func loadRecords(forKeys keys: [CacheKey], callbackQueue: DispatchQueue?, completion: @escaping (Result<[Record?], Error>) -> Void) { + self.recordsLock.lock() let records = keys.map { self.records[$0] } + self.recordsLock.unlock() DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, action: completion, result: .success(records)) @@ -19,7 +22,9 @@ public final class InMemoryNormalizedCache: NormalizedCache { public func merge(records: RecordSet, callbackQueue: DispatchQueue?, completion: @escaping (Result, Error>) -> Void) { + self.recordsLock.lock() let cacheKeys = self.records.merge(records: records) + self.recordsLock.unlock() DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, action: completion, result: .success(cacheKeys)) @@ -27,7 +32,9 @@ public final class InMemoryNormalizedCache: NormalizedCache { public func clear(callbackQueue: DispatchQueue?, completion: ((Result) -> Void)?) { + self.recordsLock.lock() self.records.clear() + self.recordsLock.unlock() guard let completion = completion else { return diff --git a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift index e6cd93e924..6f0f7ad15a 100644 --- a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift +++ b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift @@ -383,4 +383,95 @@ class FetchQueryTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } } + + func testThreadedCache() throws { + let cache = InMemoryNormalizedCache() + + let networkTransport1 = MockNetworkTransport(body: [ + "data": [ + "hero": [ + "id": "1000", + "name": "Luke Skywalker", + "__typename": "Human", + "friends": [ + ["__typename": "Human", "name": "Leia Organa", "id": "1003"], + ["__typename": "Human", "name": "Han Solo", "id": "1002"], + ] + ] + ] + ]) + + let networkTransport2 = MockNetworkTransport(body: [ + "data": [ + "hero": [ + "id": "1002", + "name": "Han Solo", + "__typename": "Human", + "friends": [ + ["__typename": "Human", "name": "Leia Organa", "id": "1003"], + ["__typename": "Human", "name": "Luke Skywalker", "id": "1000"], + ] + ] + ] + ]) + + let store = ApolloStore(cache: cache) + let store2 = ApolloStore(cache: cache) + + let client1 = ApolloClient(networkTransport: networkTransport1, store: store) + let client2 = ApolloClient(networkTransport: networkTransport2, store: store2) + + let group = DispatchGroup() + + let watcherQueue = DispatchQueue(label: "test watcher queue") + var watchers = [GraphQLQueryWatcher]() + + for _ in 0...1000 { + + group.enter() + DispatchQueue.global().async { + let watcher = + client1.watch( + query: HeroAndFriendsNamesWithIDsQuery(), cachePolicy: .returnCacheDataAndFetch) { result in + if result.value?.source == .some(.server) { + group.leave() + } + } + + watcherQueue.sync { + watchers.append(watcher) + } + } + + group.enter() + DispatchQueue.global().async { + let watcher = + client2.watch( + query: HeroAndFriendsNamesWithIDsQuery(), cachePolicy: .returnCacheDataAndFetch) { result in + if result.value?.source == .some(.server) { + group.leave() + } + } + + watcherQueue.sync { + watchers.append(watcher) + } + } + + group.enter() + DispatchQueue.global().async { + _ = client1.clearCache() { _ in + group.leave() + } + } + + } + + let expectation = self.expectation(description: "Fetching query") + group.notify(queue: .main) { + expectation.fulfill() + } + self.wait(for: [expectation], timeout: 10) + + } } From 5f4807d30a1e50fda0bbc60119b71ce2d97aa2ee Mon Sep 17 00:00:00 2001 From: Rolandas Razma Date: Tue, 21 Jan 2020 10:21:03 +0000 Subject: [PATCH 2/2] switch to NSRecursiveLock --- Sources/Apollo/InMemoryNormalizedCache.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 084c7b52f7..23b4b37315 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -2,7 +2,7 @@ import Foundation public final class InMemoryNormalizedCache: NormalizedCache { private var records: RecordSet - private let recordsLock = NSLock() + private let recordsLock = NSRecursiveLock() public init(records: RecordSet = RecordSet()) { self.records = records