Merge pull request #425 from andraskadar/bugfix/requestMapAccess

Fix: Access requestMap only on the protected queue

Accessing the content of the requestMap asynchronously on any thread can lead to concurrency issues and crashes.

These changes refactor the code without introducing changes to the behaviour to make sure that even reading of the dictionary happens on the dedicated queue.

The root cause of this is that when a task has a delegate the same code is called twice, once in didFinishCollecting and once in didCompleteWithError. The didFinishCollecting was added because in iOS 16 and async network calls the didCompleteWithError was never called, but it ended up being called twice in cases like this.

Author: Ignacio Bonafonte

Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift

@@ -549,22 +549,27 @@ public class URLSessionInstrumentation {
 
     private func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) {
         let taskId = self.idKeyForTask(task)
-        if (self.requestMap[taskId]?.request) != nil {
-            /// Code for instrumenting colletion should be written here
-            var requestState: NetworkRequestState?
-            queue.sync {
-                requestState = requestMap[taskId]
-                if requestState != nil {
-                    requestMap[taskId] = nil
-                }
-            }
-            if let error = task.error {
-                let status = (task.response as? HTTPURLResponse)?.statusCode ?? 0
-                URLSessionLogger.logError(error, dataOrFile: requestState?.dataProcessed, statusCode: status, instrumentation: self, sessionTaskId: taskId)
-            } else if let response = task.response {
-                URLSessionLogger.logResponse(response, dataOrFile: requestState?.dataProcessed, instrumentation: self, sessionTaskId: taskId)
+
+        var requestState: NetworkRequestState?
+        queue.sync {
+            requestState = requestMap[taskId]
+
+            if requestState?.request != nil {
+                requestMap[taskId] = nil
             }
         }
+
+        guard requestState?.request != nil else {
+            return
+        }
+
+        /// Code for instrumenting colletion should be written here
+        if let error = task.error {
+            let status = (task.response as? HTTPURLResponse)?.statusCode ?? 0
+            URLSessionLogger.logError(error, dataOrFile: requestState?.dataProcessed, statusCode: status, instrumentation: self, sessionTaskId: taskId)
+        } else if let response = task.response {
+            URLSessionLogger.logResponse(response, dataOrFile: requestState?.dataProcessed, instrumentation: self, sessionTaskId: taskId)
+        }
     }
 
     private func urlSessionTaskWillResume(_ task: URLSessionTask) {