AddressCacheStore: use simple lock

4 files changed, 61 insertions, 74 deletions

diff --git a/ios/MullvadVPN/AddressCache/AddressCacheStore.swift b/ios/MullvadVPN/AddressCache/AddressCacheStore.swift
index 37bf2f2cd7..c64a8cf163 100644
--- a/ios/MullvadVPN/AddressCache/AddressCacheStore.swift
+++ b/ios/MullvadVPN/AddressCache/AddressCacheStore.swift
@@ -76,8 +76,8 @@ extension AddressCache {
         /// The location of pre-bundled address cache file.
         private let prebundledCacheFileURL: URL
 
-        /// Queue used for synchronizing access to instance members.
-        private let stateQueue = DispatchQueue(label: "AddressCacheStoreQueue")
+        /// Lock used for synchronizing access to instance members.
+        private let nslock = NSLock()
 
         /// Designated initializer
         init(cacheFileURL: URL, prebundledCacheFileURL: URL) {
@@ -105,7 +105,7 @@ extension AddressCache {
 
                         logger.debug("Persist address list read from bundle.")
 
-                        if case .failure(let error) = self.writeToDisk() {
+                        if case .failure(let error) = writeToDisk() {
                             logger.error(chainedError: error, message: "Failed to persist address cache after reading it from bundle.")
                         }
                     }
@@ -122,77 +122,69 @@ extension AddressCache {
             }
         }
 
-        func getCurrentEndpoint(_ completionHandler: @escaping (AnyIPEndpoint) -> Void) {
-            stateQueue.async {
-                let currentEndpoint = self.cachedAddresses.endpoints.first!
-
-                completionHandler(currentEndpoint)
-            }
+        func getCurrentEndpoint() -> AnyIPEndpoint {
+            nslock.lock()
+            defer { nslock.unlock() }
+            return cachedAddresses.endpoints.first!
         }
 
-        func selectNextEndpoint(_ failedEndpoint: AnyIPEndpoint, completionHandler: @escaping (AnyIPEndpoint) -> Void) {
-            stateQueue.async {
-                var currentEndpoint = self.cachedAddresses.endpoints.first!
+        func selectNextEndpoint(_ failedEndpoint: AnyIPEndpoint) -> AnyIPEndpoint {
+            nslock.lock()
+            defer { nslock.unlock() }
 
-                if failedEndpoint == currentEndpoint {
-                    self.cachedAddresses.endpoints.removeFirst()
-                    self.cachedAddresses.endpoints.append(failedEndpoint)
+            var currentEndpoint = cachedAddresses.endpoints.first!
 
-                    currentEndpoint = self.cachedAddresses.endpoints.first!
+            if failedEndpoint == currentEndpoint {
+                cachedAddresses.endpoints.removeFirst()
+                cachedAddresses.endpoints.append(failedEndpoint)
 
-                    self.logger.debug("Failed to communicate using \(failedEndpoint). Next endpoint: \(currentEndpoint)")
+                currentEndpoint = cachedAddresses.endpoints.first!
 
-                    if case .failure(let error) = self.writeToDisk() {
-                        self.logger.error(chainedError: error, message: "Failed to write address cache after selecting next endpoint.")
-                    }
-                }
+                logger.debug("Failed to communicate using \(failedEndpoint). Next endpoint: \(currentEndpoint)")
 
-                completionHandler(currentEndpoint)
+                if case .failure(let error) = writeToDisk() {
+                    logger.error(chainedError: error, message: "Failed to write address cache after selecting next endpoint.")
+                }
             }
+
+            return currentEndpoint
         }
 
-        func setEndpoints(_ endpoints: [AnyIPEndpoint], completionHandler: @escaping (AddressCache.StoreError?) -> Void) {
-            stateQueue.async {
-                guard !endpoints.isEmpty else {
-                    completionHandler(.emptyAddressList)
-                    return
-                }
+        func setEndpoints(_ endpoints: [AnyIPEndpoint]) -> Result<Void, AddressCache.StoreError> {
+            nslock.lock()
+            defer { nslock.unlock() }
 
-                if Set(self.cachedAddresses.endpoints) == Set(endpoints) {
-                    self.cachedAddresses.updatedAt = Date()
-                } else {
-                    // Shuffle new endpoints
-                    var newEndpoints = endpoints.shuffled()
+            guard !endpoints.isEmpty else {
+                return .failure(.emptyAddressList)
+            }
 
-                    // Move current endpoint to the top of the list
-                    let currentEndpoint = self.cachedAddresses.endpoints.first!
-                    if let index = newEndpoints.firstIndex(of: currentEndpoint) {
-                        newEndpoints.remove(at: index)
-                        newEndpoints.insert(currentEndpoint, at: 0)
-                    }
+            if Set(cachedAddresses.endpoints) == Set(endpoints) {
+                cachedAddresses.updatedAt = Date()
+            } else {
+                // Shuffle new endpoints
+                var newEndpoints = endpoints.shuffled()
 
-                    self.cachedAddresses = CachedAddresses(
-                        updatedAt: Date(),
-                        endpoints: newEndpoints
-                    )
+                // Move current endpoint to the top of the list
+                let currentEndpoint = cachedAddresses.endpoints.first!
+                if let index = newEndpoints.firstIndex(of: currentEndpoint) {
+                    newEndpoints.remove(at: index)
+                    newEndpoints.insert(currentEndpoint, at: 0)
                 }
 
-                let writeResult = self.writeToDisk()
-
-                completionHandler(writeResult.error)
+                cachedAddresses = CachedAddresses(
+                    updatedAt: Date(),
+                    endpoints: newEndpoints
+                )
             }
-        }
 
-        func getLastUpdateDate(_ completionHandler: @escaping (Date) -> Void) {
-            stateQueue.async {
-                completionHandler(self.cachedAddresses.updatedAt)
-            }
+            return writeToDisk()
         }
 
-        func getLastUpdateDateAndWait() -> Date {
-            return stateQueue.sync {
-                return self.cachedAddresses.updatedAt
-            }
+        func getLastUpdateDate() -> Date {
+            nslock.lock()
+            defer { nslock.unlock() }
+
+            return cachedAddresses.updatedAt
         }
 
         private func readFromCacheLocationWithFallback() -> Result<ReadResult, AddressCache.StoreError> {
diff --git a/ios/MullvadVPN/AddressCache/AddressCacheTracker.swift b/ios/MullvadVPN/AddressCache/AddressCacheTracker.swift
index 390fc31b4c..e151de9997 100644
--- a/ios/MullvadVPN/AddressCache/AddressCacheTracker.swift
+++ b/ios/MullvadVPN/AddressCache/AddressCacheTracker.swift
@@ -138,7 +138,7 @@ extension AddressCache {
             if let lastFailureAttemptDate = lastFailureAttemptDate {
                 return Date(timeInterval: Self.retryInterval, since: lastFailureAttemptDate)
             } else {
-                let updatedAt = store.getLastUpdateDateAndWait()
+                let updatedAt = store.getLastUpdateDate()
 
                 return Date(timeInterval: Self.updateInterval, since: updatedAt)
             }
diff --git a/ios/MullvadVPN/AddressCache/UpdateAddressCacheOperation.swift b/ios/MullvadVPN/AddressCache/UpdateAddressCacheOperation.swift
index 2b2d42ac33..1ce039d877 100644
--- a/ios/MullvadVPN/AddressCache/UpdateAddressCacheOperation.swift
+++ b/ios/MullvadVPN/AddressCache/UpdateAddressCacheOperation.swift
@@ -56,7 +56,7 @@ extension AddressCache {
                 return
             }
 
-            let lastUpdate = store.getLastUpdateDateAndWait()
+            let lastUpdate = store.getLastUpdateDate()
             let nextUpdate = Date(timeInterval: updateInterval, since: lastUpdate)
 
             guard nextUpdate <= Date() else {
@@ -74,12 +74,11 @@ extension AddressCache {
         private func handleResponse(_ completion: OperationCompletion<[AnyIPEndpoint], REST.Error>) {
             switch completion {
             case .success(let newEndpoints):
-                self.store.setEndpoints(newEndpoints) { error in
-                    if let error = error {
-                        self.finish(completion: .failure(error))
-                    } else {
-                        self.finish(completion: .success(.finished))
-                    }
+                switch store.setEndpoints(newEndpoints) {
+                case .success:
+                    self.finish(completion: .success(.finished))
+                case .failure(let error):
+                    self.finish(completion: .failure(error))
                 }
 
             case .failure(let error):
diff --git a/ios/MullvadVPN/REST/RESTNetworkOperation.swift b/ios/MullvadVPN/REST/RESTNetworkOperation.swift
index cc20c526b0..f2760bcbcf 100644
--- a/ios/MullvadVPN/REST/RESTNetworkOperation.swift
+++ b/ios/MullvadVPN/REST/RESTNetworkOperation.swift
@@ -67,12 +67,10 @@ extension REST {
                 }
 
                 // Get current endpoint
-                self.addressCacheStore.getCurrentEndpoint { endpoint in
-                    DispatchQueue.main.async {
-                        self.sendRequest(endpoint: endpoint) { [weak self] completion in
-                            self?.finish(completion: completion)
-                        }
-                    }
+                let endpoint = self.addressCacheStore.getCurrentEndpoint()
+
+                self.sendRequest(endpoint: endpoint) { [weak self] completion in
+                    self?.finish(completion: completion)
                 }
             }
         }
@@ -116,11 +114,9 @@ extension REST {
             switch Self.evaluateError(error) {
             case .useNextEndpoint:
                 // Pick next endpoint in the event of network error
-                addressCacheStore.selectNextEndpoint(endpoint) { nextEndpoint in
-                    DispatchQueue.main.async {
-                        self.retryRequest(endpoint: nextEndpoint, previousResult: result, completionHandler: completionHandler)
-                    }
-                }
+                let nextEndpoint = addressCacheStore.selectNextEndpoint(endpoint)
+                
+                retryRequest(endpoint: nextEndpoint, previousResult: result, completionHandler: completionHandler)
 
             case .useCurrentEndpoint:
                 // Retry request using the same endpoint otherwise