Fix threading issue: activeRequests could be accessed off of the rootQueue. (#3179)

* Fix queueing for request setup methods.

* Update comment.

* Update GitHub to use Xcode 11.4.1.

.github/workflows/ci.yml

@@ -23,7 +23,7 @@ jobs:
     name: Test macOS (5.2)
     runs-on: macOS-latest
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     steps:
       - uses: actions/checkout@v2
       - name: macOS
@@ -32,7 +32,7 @@ jobs:
     name: Test Catalyst 
     runs-on: macOS-latest
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     steps:
       - uses: actions/checkout@v2
       - name: Catalyst
@@ -41,10 +41,10 @@ jobs:
     name: Test iOS 
     runs-on: macOS-latest
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
-        destination: ["OS=13.4,name=iPhone 11 Pro"] #, "OS=12.4,name=iPhone XS", "OS=11.4,name=iPhone X", "OS=10.3.1,name=iPhone SE"]
+        destination: ["OS=13.4.1,name=iPhone 11 Pro"] #, "OS=12.4,name=iPhone XS", "OS=11.4,name=iPhone X", "OS=10.3.1,name=iPhone SE"]
     steps:
       - uses: actions/checkout@v2
       - name: iOS - ${{ matrix.destination }}
@@ -53,7 +53,7 @@ jobs:
     name: Test tvOS 
     runs-on: macOS-latest
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
         destination: ["OS=13.4,name=Apple TV 4K"] #, "OS=11.4,name=Apple TV 4K", "OS=10.2,name=Apple TV 1080p"]
@@ -65,7 +65,7 @@ jobs:
     name: Build watchOS
     runs-on: macOS-latest
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
         destination: ["OS=6.2,name=Apple Watch Series 5 - 44mm"] #, "OS=4.2,name=Apple Watch Series 3 - 42mm", "OS=3.2,name=Apple Watch Series 2 - 42mm"]
@@ -77,7 +77,7 @@ jobs:
     name: Test with SPM
     runs-on: macOS-latest    
     env: 
-      DEVELOPER_DIR: /Applications/Xcode_11.4.app/Contents/Developer
+      DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     steps:
       - uses: actions/checkout@v2
       - name: SPM Test

Source/Session.swift

@@ -963,75 +963,67 @@ open class Session {
 
     // MARK: Perform
 
-    /// Perform `Request`.
-    ///
-    /// - Note: Called during retry.
+    /// Starts performing the provided `Request`.
     ///
     /// - Parameter request: The `Request` to perform.
     func perform(_ request: Request) {
-        // Leaf types must come first, otherwise they will cast as their superclass.
-        switch request {
-        case let r as UploadRequest: perform(r) // UploadRequest must come before DataRequest due to subtype relationship.
-        case let r as DataRequest: perform(r)
-        case let r as DownloadRequest: perform(r)
-        case let r as DataStreamRequest: perform(r)
-        default: fatalError("Attempted to perform unsupported Request subclass: \(type(of: request))")
-        }
-    }
-
-    func perform(_ request: DataRequest) {
-        requestQueue.async {
+        rootQueue.async {
             guard !request.isCancelled else { return }
 
             self.activeRequests.insert(request)
 
-            self.performSetupOperations(for: request, convertible: request.convertible)
+            self.requestQueue.async {
+                // Leaf types must come first, otherwise they will cast as their superclass.
+                switch request {
+                case let r as UploadRequest: self.performUploadRequest(r) // UploadRequest must come before DataRequest due to subtype relationship.
+                case let r as DataRequest: self.performDataRequest(r)
+                case let r as DownloadRequest: self.performDownloadRequest(r)
+                case let r as DataStreamRequest: self.performDataStreamRequest(r)
+                default: fatalError("Attempted to perform unsupported Request subclass: \(type(of: request))")
+                }
+            }
         }
     }
 
-    func perform(_ request: DataStreamRequest) {
-        requestQueue.async {
-            guard !request.isCancelled else { return }
-
-            self.activeRequests.insert(request)
+    func performDataRequest(_ request: DataRequest) {
+        dispatchPrecondition(condition: .onQueue(requestQueue))
 
-            self.performSetupOperations(for: request, convertible: request.convertible)
-        }
+        performSetupOperations(for: request, convertible: request.convertible)
     }
 
-    func perform(_ request: UploadRequest) {
-        requestQueue.async {
-            guard !request.isCancelled else { return }
+    func performDataStreamRequest(_ request: DataStreamRequest) {
+        dispatchPrecondition(condition: .onQueue(requestQueue))
 
-            self.activeRequests.insert(request)
+        performSetupOperations(for: request, convertible: request.convertible)
+    }
 
-            do {
-                let uploadable = try request.upload.createUploadable()
-                self.rootQueue.async { request.didCreateUploadable(uploadable) }
+    func performUploadRequest(_ request: UploadRequest) {
+        dispatchPrecondition(condition: .onQueue(requestQueue))
 
-                self.performSetupOperations(for: request, convertible: request.convertible)
-            } catch {
-                self.rootQueue.async { request.didFailToCreateUploadable(with: error.asAFError(or: .createUploadableFailed(error: error))) }
-            }
+        do {
+            let uploadable = try request.upload.createUploadable()
+            rootQueue.async { request.didCreateUploadable(uploadable) }
+
+            performSetupOperations(for: request, convertible: request.convertible)
+        } catch {
+            rootQueue.async { request.didFailToCreateUploadable(with: error.asAFError(or: .createUploadableFailed(error: error))) }
         }
     }
 
-    func perform(_ request: DownloadRequest) {
-        requestQueue.async {
-            guard !request.isCancelled else { return }
-
-            self.activeRequests.insert(request)
+    func performDownloadRequest(_ request: DownloadRequest) {
+        dispatchPrecondition(condition: .onQueue(requestQueue))
 
-            switch request.downloadable {
-            case let .request(convertible):
-                self.performSetupOperations(for: request, convertible: convertible)
-            case let .resumeData(resumeData):
-                self.rootQueue.async { self.didReceiveResumeData(resumeData, for: request) }
-            }
+        switch request.downloadable {
+        case let .request(convertible):
+            performSetupOperations(for: request, convertible: convertible)
+        case let .resumeData(resumeData):
+            rootQueue.async { self.didReceiveResumeData(resumeData, for: request) }
         }
     }
 
     func performSetupOperations(for request: Request, convertible: URLRequestConvertible) {
+        dispatchPrecondition(condition: .onQueue(requestQueue))
+
         let initialRequest: URLRequest
 
         do {
@@ -1069,6 +1061,8 @@ open class Session {
     // MARK: - Task Handling
 
     func didCreateURLRequest(_ urlRequest: URLRequest, for request: Request) {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         request.didCreateURLRequest(urlRequest)
 
         guard !request.isCancelled else { return }
@@ -1081,6 +1075,8 @@ open class Session {
     }
 
     func didReceiveResumeData(_ data: Data, for request: DownloadRequest) {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         guard !request.isCancelled else { return }
 
         let task = request.task(forResumeData: data, using: session)
@@ -1091,6 +1087,8 @@ open class Session {
     }
 
     func updateStatesForTask(_ task: URLSessionTask, request: Request) {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         request.withState { state in
             switch state {
             case .initialized, .finished:

Tests/RequestTests.swift

@@ -200,7 +200,7 @@ final class RequestResponseTestCase: BaseTestCase {
         XCTAssertEqual(response?.result.isSuccess, true)
     }
 
-    func testThatRequestsWorksWithRequestAndSerializationQueue() {
+    func testThatRequestsWorksWithRequestAndSerializationQueues() {
         // Given
         let requestQueue = DispatchQueue(label: "org.alamofire.testRequestQueue")
         let serializationQueue = DispatchQueue(label: "org.alamofire.testSerializationQueue")
@@ -220,6 +220,31 @@ final class RequestResponseTestCase: BaseTestCase {
         XCTAssertEqual(response?.result.isSuccess, true)
     }
 
+    func testThatRequestsWorksWithConcurrentRequestAndSerializationQueues() {
+        // Given
+        let requestQueue = DispatchQueue(label: "org.alamofire.testRequestQueue", attributes: .concurrent)
+        let serializationQueue = DispatchQueue(label: "org.alamofire.testSerializationQueue", attributes: .concurrent)
+        let session = Session(requestQueue: requestQueue, serializationQueue: serializationQueue)
+        let count = 10
+        let expectation = self.expectation(description: "request should complete")
+        expectation.expectedFulfillmentCount = count
+        var responses: [DataResponse<Any, AFError>] = []
+
+        // When
+        DispatchQueue.concurrentPerform(iterations: count) { _ in
+            session.request("https://httpbin.org/get").responseJSON { resp in
+                responses.append(resp)
+                expectation.fulfill()
+            }
+        }
+
+        waitForExpectations(timeout: timeout, handler: nil)
+
+        // Then
+        XCTAssertEqual(responses.count, count)
+        XCTAssertTrue(responses.allSatisfy { $0.result.isSuccess })
+    }
+
     // MARK: Encodable Parameters
 
     func testThatRequestsCanPassEncodableParametersAsJSONBodyData() {