fix: multithreading handling

MetaMask · Sep 14, 2024 · 64b8080 · 64b8080
1 parent 0399a53
commit 64b8080
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 25 deletions.
diff --git a/Sources/metamask-ios-sdk/Classes/API/InfuraProvider.swift b/Sources/metamask-ios-sdk/Classes/API/InfuraProvider.swift
@@ -109,7 +109,6 @@ public class ReadOnlyRPCProvider {
                             params: Any = "",
                             chainId: String,
                             appMetadata: AppMetadata) async -> Any? {
-        Logging.log("ReadOnlyRPCProvider:: Sending request \(request.method) on chain \(chainId) via Infura API")
 
         let params: [String: Any] = [
             "method": request.method,
@@ -122,6 +121,8 @@ public class ReadOnlyRPCProvider {
             Logging.error("ReadOnlyRPCProvider:: Infura endpoint for chainId \(chainId) is not available")
             return nil
         }
+
+        Logging.log("ReadOnlyRPCProvider:: Sending request \(request.method) on chain \(chainId) using endpoint \(endpoint) via Infura API")
 
         let devicePlatformInfo = DeviceInfo.platformDescription
         network.addHeaders([

diff --git a/Sources/metamask-ios-sdk/Classes/API/Network.swift b/Sources/metamask-ios-sdk/Classes/API/Network.swift
@@ -14,11 +14,25 @@ public protocol Networking: ObservableObject {
 
 public class Network: Networking {
     public init() {}
+
+    private let queue = DispatchQueue(label: "headers.queue")
 
     private var additionalHeaders: [String: String] = [
         "Accept": "application/json",
         "Content-Type": "application/json"
     ]
+
+    func getAdditionalHeaders() -> [String: String] {
+        return queue.sync { [weak self] in
+            return self?.additionalHeaders ?? [:]
+        }
+    }
+
+    func mergeHeaders(_ headers: [String: String]) {
+        queue.sync {
+            additionalHeaders.merge(headers) { (_, new) in new }
+        }
+    }
 
     public func fetch<T: Decodable>(_ Type: T.Type, endpoint: Endpoint) async throws -> T {
         guard let url = URL(string: endpoint.url) else {
@@ -52,12 +66,12 @@ public class Network: Networking {
     }
 
     public func addHeaders(_ headers: [String: String]) {
-        additionalHeaders.merge(headers) { (_, new) in new }
+        mergeHeaders(headers)
     }
 
     private func request(for url: URL) -> URLRequest {
         var request = URLRequest(url: url)
-        for (key, value) in additionalHeaders {
+        for (key, value) in getAdditionalHeaders() {
             request.addValue(value, forHTTPHeaderField: key)
         }
 

diff --git a/Sources/metamask-ios-sdk/Classes/Analytics/Event.swift b/Sources/metamask-ios-sdk/Classes/Analytics/Event.swift
@@ -11,6 +11,7 @@ public enum Event: String {
     case connectionAuthorised = "sdk_connection_authorized"
     case connectionRejected = "sdk_connection_rejected"
     case disconnected = "sdk_disconnected"
+    case connectionTerminated = "sdk_connection_terminated"
 
     var name: String {
         rawValue

diff --git a/Sources/metamask-ios-sdk/Classes/Ethereum/Ethereum.swift b/Sources/metamask-ios-sdk/Classes/Ethereum/Ethereum.swift
@@ -16,8 +16,12 @@ protocol EthereumEventsDelegate: AnyObject {
 public class Ethereum {
     static let CONNECTION_ID = TimestampGenerator.timestamp()
     static let BATCH_CONNECTION_ID = TimestampGenerator.timestamp()
+
     var submittedRequests: [String: SubmittedRequest] = [:]
+    private let queue = DispatchQueue(label: "submittedRequests.queue")
+
     private var cancellables: Set<AnyCancellable> = []
+    private let cancellablesLock = NSRecursiveLock()
 
     let readOnlyRPCProvider: ReadOnlyRPCProvider
 
@@ -85,7 +89,7 @@ public class Ethereum {
     }
 
     private func fetchCachedSession() {
-        if 
+        if
             let account = store.string(for: ACCOUNT_KEY),
             let chainId = store.string(for: CHAINID_KEY)
         {
@@ -126,6 +130,42 @@ public class Ethereum {
         appMetadata = metadata
         commClient.appMetadata = metadata
     }
+
+    func addRequest(_ submittedRequest: SubmittedRequest, id: String) {
+        queue.async { [weak self] in
+            self?.submittedRequests[id] = submittedRequest
+        }
+    }
+
+    func getAllRequests() -> [String: SubmittedRequest] {
+        return queue.sync { [weak self] in
+            return self?.submittedRequests ?? [:]
+        }
+    }
+
+    func getRequest(id: String) -> SubmittedRequest? {
+        return queue.sync { [weak self] in
+            return self?.submittedRequests[id]
+        }
+    }
+
+    func removeRequest(id: String) {
+        queue.async { [weak self] in
+            self?.submittedRequests.removeValue(forKey: id)
+        }
+    }
+
+    func removeAllRequests() {
+        queue.async { [weak self] in
+            self?.submittedRequests.removeAll()
+        }
+    }
+
+    private func syncCancellables() -> Set<AnyCancellable> {
+        cancellablesLock.sync {
+            return cancellables
+        }
+    }
 
     // MARK: Session Management
 
@@ -141,8 +181,8 @@ public class Ethereum {
         }
 
         let submittedRequest = SubmittedRequest(method: "")
-        submittedRequests[Ethereum.CONNECTION_ID] = submittedRequest
-        let publisher = submittedRequests[Ethereum.CONNECTION_ID]?.publisher
+        addRequest(submittedRequest, id: Ethereum.CONNECTION_ID)
+        let publisher = getRequest(id: Ethereum.CONNECTION_ID)?.publisher
 
         return publisher
     }
@@ -153,7 +193,7 @@ public class Ethereum {
         }
 
         return await withCheckedContinuation { continuation in
-            publisher
+            let cancellable = publisher
                 .tryMap { output in
                     // remove nil and NSNUll values in result
                     if  let resultArray = output as? [Any?] {
@@ -182,7 +222,11 @@ public class Ethereum {
                     }
                 }, receiveValue: { result in
                     continuation.resume(returning: .success(result))
-                }).store(in: &cancellables)
+                })
+
+            cancellablesLock.sync {
+                cancellables.insert(cancellable)
+            }
         }
     }
 
@@ -216,8 +260,8 @@ public class Ethereum {
         }
 
         let submittedRequest = SubmittedRequest(method: connectSignRequest.method)
-        submittedRequests[connectSignRequest.id] = submittedRequest
-        let publisher = submittedRequests[connectSignRequest.id]?.publisher
+        addRequest(submittedRequest, id: connectSignRequest.id)
+        let publisher = getRequest(id: connectSignRequest.id)?.publisher
 
         commClient.connect(with: requestJson)
 
@@ -262,8 +306,8 @@ public class Ethereum {
             }
         case .deeplinking:
             let submittedRequest = SubmittedRequest(method: connectWithRequest.method)
-            submittedRequests[connectWithRequest.id] = submittedRequest
-            let publisher = submittedRequests[connectWithRequest.id]?.publisher
+            addRequest(submittedRequest, id: connectWithRequest.id)
+            let publisher = getRequest(id: connectWithRequest.id)?.publisher
 
             // React Native SDK has request params as Data
             if let paramsData = req.params as? Data {
@@ -421,14 +465,14 @@ public class Ethereum {
 
     func terminateConnection() {
         if connected {
-            track?(.connectionRejected, [:])
+            track?(.connectionTerminated, [:])
         }
 
         let error = RequestError(from: ["message": "The connection request has been rejected"])
-        submittedRequests.forEach { key, _ in
-            submittedRequests[key]?.error(error)
+        getAllRequests().forEach { key, _ in
+            getRequest(id: key)?.error(error)
         }
-        submittedRequests.removeAll()
+        removeAllRequests()
         clearSession()
     }
 
@@ -534,8 +578,8 @@ public class Ethereum {
         )
 
         let submittedRequest = SubmittedRequest(method: requestAccountsRequest.method)
-        submittedRequests[requestAccountsRequest.id] = submittedRequest
-        let publisher = submittedRequests[requestAccountsRequest.id]?.publisher
+        addRequest(submittedRequest, id: requestAccountsRequest.id)
+        let publisher = getRequest(id: requestAccountsRequest.id)?.publisher
 
         commClient.addRequest { [weak self] in
             self?.sendRequest(requestAccountsRequest)
@@ -562,8 +606,9 @@ public class Ethereum {
         } else {
             let id = request.id
             let submittedRequest = SubmittedRequest(method: request.method)
-            submittedRequests[id] = submittedRequest
-            let publisher = submittedRequests[id]?.publisher
+            addRequest(submittedRequest, id: id)
+
+            let publisher = getRequest(id: id)?.publisher
 
             if connected || !account.isEmpty {
                 connected = true
@@ -649,13 +694,13 @@ public class Ethereum {
     }
 
     func sendResult(_ result: Any, id: String) {
-        submittedRequests[id]?.send(result)
-        submittedRequests.removeValue(forKey: id)
+        getRequest(id: id)?.send(result)
+        removeRequest(id: id)
     }
 
     func sendError(_ error: RequestError, id: String) {
-        submittedRequests[id]?.error(error)
-        submittedRequests.removeValue(forKey: id)
+        getRequest(id: id)?.error(error)
+        removeRequest(id: id)
 
         if error.codeType == .unauthorisedRequest {
             clearSession()
@@ -676,7 +721,7 @@ public class Ethereum {
     }
 
     func receiveResponse(_ data: [String: Any], id: String) {
-        guard let request = submittedRequests[id] else { return }
+        guard let request = getRequest(id: id) else { return }
 
         track?(.sdkRpcRequestDone, [
             "from": "mobile",

diff --git a/Sources/metamask-ios-sdk/Classes/Extensions/NSRecursiveLock.swift b/Sources/metamask-ios-sdk/Classes/Extensions/NSRecursiveLock.swift
@@ -0,0 +1,14 @@
+//
+//  NSRecursiveLock.swift
+//
+
+import Foundation
+
+extension NSRecursiveLock {
+  @inlinable @discardableResult
+  func sync<Value>(_ work: () -> Value) -> Value {
+    lock()
+    defer { unlock() }
+    return work()
+  }
+}