Просмотр исходного кода

Work towards investigating mass cancellation failures.

Jon Shier 6 лет назад
Родитель
Сommit
ac18b431b2
4 измененных файлов с 87 добавлено и 11 удалено
  1. 12 3
      Source/Request.swift
  2. 3 3
      Source/RequestTaskMap.swift
  3. 32 1
      Source/Session.swift
  4. 40 4
      Tests/SessionTests.swift

+ 12 - 3
Source/Request.swift

@@ -65,7 +65,7 @@ public class Request {
 
     // MARK: - Initial State
 
-    /// `UUID` prividing a unique identifier for the `Request`, used in the `Hashable` and `Equatable` conformances.
+    /// `UUID` providing a unique identifier for the `Request`, used in the `Hashable` and `Equatable` conformances.
     public let id: UUID
     /// The serial queue for all internal async actions.
     public let underlyingQueue: DispatchQueue
@@ -343,6 +343,8 @@ public class Request {
     ///
     /// - Parameter task: The `URLSessionTask` created.
     func didCreateTask(_ task: URLSessionTask) {
+        //assert(protectedMutableState.directValue.state != .cancelled)
+        
         protectedMutableState.write { $0.tasks.append(task) }
 
         eventMonitor?.request(self, didCreateTask: task)
@@ -452,10 +454,15 @@ public class Request {
         }
     }
 
+    var finishedCalled = false
     /// Finishes this `Request` and starts the response serializers.
     ///
     /// - Parameter error: The possible `Error` with which the instance will finish.
     func finish(error: AFError? = nil) {
+        //assert(!finishedCalled)
+        
+        finishedCalled = true
+        
         if let error = error { self.error = error }
 
         // Start response handlers
@@ -602,11 +609,13 @@ public class Request {
             mutableState.state = .cancelled
 
             underlyingQueue.async { self.didCancel() }
-
-            guard let task = mutableState.tasks.last, task.state != .completed else {
+            
+            guard let task = mutableState.tasks.last else {
                 underlyingQueue.async { self.finish() }
                 return
             }
+            
+            guard ![.canceling, .completed].contains(task.state) else { return }
 
             // Resume to ensure metrics are gathered.
             task.resume()

+ 3 - 3
Source/RequestTaskMap.swift

@@ -26,9 +26,9 @@ import Foundation
 
 /// A type that maintains a two way, one to one map of `URLSessionTask`s to `Request`s.
 struct RequestTaskMap {
-    private var tasksToRequests: [URLSessionTask: Request]
-    private var requestsToTasks: [Request: URLSessionTask]
-    private var taskEvents: [URLSessionTask: (completed: Bool, metricsGathered: Bool)]
+    var tasksToRequests: [URLSessionTask: Request]
+    var requestsToTasks: [Request: URLSessionTask]
+    var taskEvents: [URLSessionTask: (completed: Bool, metricsGathered: Bool)]
 
     var requests: [Request] {
         return Array(tasksToRequests.values)

+ 32 - 1
Source/Session.swift

@@ -908,6 +908,37 @@ open class Session {
         request.didCreateTask(task)
 
         updateStatesForTask(task, request: request)
+        
+//        request.withState { state in
+//            let task = request.task(for: urlRequest, using: session)
+//            requestTaskMap[request] = task
+//            rootQueue.async {
+//                request.didCreateTask(task)
+//                request.withState { innerState in
+//                    switch (self.startRequestsImmediately, innerState) {
+//                    case (true, .initialized):
+//                        self.rootQueue.async { request.resume() }
+//                    case (false, .initialized):
+//                        // Do nothing.
+//                        break
+//                    case (_, .resumed):
+//                        task.resume()
+//                        self.rootQueue.async { request.didResumeTask(task) }
+//                    case (_, .suspended):
+//                        task.suspend()
+//                        self.rootQueue.async { request.didSuspendTask(task) }
+//                    case (_, .cancelled):
+//                        // Resume to ensure metrics are gathered.
+//                        task.resume()
+//                        task.cancel()
+//                        self.rootQueue.async { request.didCancelTask(task) }
+//                    case (_, .finished):
+//                        // Do nothing
+//                        break
+//                    }
+//                }
+//            }
+//        }
     }
 
     func didReceiveResumeData(_ data: Data, for request: DownloadRequest) {
@@ -967,7 +998,7 @@ open class Session {
     // MARK: - Invalidation
 
     func finishRequestsForDeinit() {
-        requestTaskMap.requests.forEach { $0.finish(error: AFError.sessionDeinitialized) }
+//        requestTaskMap.requests.forEach { $0.finish(error: AFError.sessionDeinitialized) }
     }
 }
 

+ 40 - 4
Tests/SessionTests.swift

@@ -1480,11 +1480,46 @@ final class SessionTestCase: BaseTestCase {
 
 // MARK: -
 
+final class EventAccumulator: EventMonitor {
+    var didCreateTasks: [(Request, URLSessionTask)] = []
+    var didCancelTasks: [(Request, URLSessionTask)] = []
+    var requestDidFinish: [Request] = []
+    var requestDidGatherMetrics: [Request] = []
+    var sessionDidGatherMetrics: [URLSessionTask] = []
+    
+    func request(_ request: Request, didCreateTask task: URLSessionTask) {
+        didCreateTasks.append((request, task))
+    }
+    
+    func request(_ request: Request, didCancelTask task: URLSessionTask) {
+        didCancelTasks.append((request, task))
+    }
+    
+    func requestDidFinish(_ request: Request) {
+        requestDidFinish.append(request)
+    }
+    
+    func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) {
+        sessionDidGatherMetrics.append(task)
+    }
+    
+    func request(_ request: Request, didGatherMetrics metrics: URLSessionTaskMetrics) {
+        requestDidGatherMetrics.append(request)
+    }
+}
+
 final class SessionCancellationTestCase: BaseTestCase {
+    func testMany() {
+        for _ in 0..<100 {
+            testThatAutomaticallyResumedRequestsCanBeMassCancelled()
+        }
+    }
+    
     func testThatAutomaticallyResumedRequestsCanBeMassCancelled() {
         // Given
-        let count = 100
-        let session = Session()
+        let count = 1000
+        let accumulator = EventAccumulator()
+        let session = Session(eventMonitors: [accumulator])
         var responses: [DataResponse<Data?, AFError>] = []
         let completion = expectation(description: "all requests should finish")
         completion.expectedFulfillmentCount = count
@@ -1504,17 +1539,18 @@ final class SessionCancellationTestCase: BaseTestCase {
 
         waitForExpectations(timeout: timeout)
 
+        print("")
         // Then
         XCTAssertTrue(responses.allSatisfy { $0.error?.isExplicitlyCancelledError == true })
         assert(on: session.rootQueue) {
-            XCTAssertTrue(session.requestTaskMap.isEmpty, "requestTaskMap should be empty but has \(session.requestTaskMap.count) items")
+            XCTAssertTrue(session.requestTaskMap.isEmpty, "requestTaskMap should be empty but has \(session.requestTaskMap.count) items, id: \(session.requestTaskMap.tasksToRequests.first?.value.id)")
             XCTAssertTrue(session.activeRequests.isEmpty, "activeRequests should be empty but has \(session.activeRequests.count) items")
         }
     }
 
     func testThatManuallyResumedRequestsCanBeMassCancelled() {
         // Given
-        let count = 100
+        let count = 1000
         let session = Session(startRequestsImmediately: false)
         let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
         var responses: [DataResponse<Data?, AFError>] = []