소스 검색

Merge pull request #379 from cnoon/fixes/task_delegate_deinit

Bug fix for deinit race condition on task delegate queue.
Christian Noon 10 년 전
부모
커밋
83774b0f9b
2개의 변경된 파일34개의 추가작업 그리고 11개의 파일을 삭제
  1. 18 11
      Source/Alamofire.swift
  2. 16 0
      Tests/ManagerTests.swift

+ 18 - 11
Source/Alamofire.swift

@@ -219,8 +219,6 @@ extension NSURLRequest: URLRequestConvertible {
 
 /**
     Responsible for creating and managing `Request` objects, as well as their underlying `NSURLSession`.
-
-    When finished with a manager, be sure to call either `session.finishTasksAndInvalidate()` or `session.invalidateAndCancel()` before deinitialization.
 */
 public class Manager {
 
@@ -308,6 +306,10 @@ public class Manager {
         }
     }
 
+    deinit {
+        self.session.invalidateAndCancel()
+    }
+
     // MARK: -
 
     /**
@@ -702,7 +704,7 @@ public class Request {
         :returns: The request.
     */
     public func response(queue: dispatch_queue_t? = nil, serializer: Serializer, completionHandler: (NSURLRequest, NSHTTPURLResponse?, AnyObject?, NSError?) -> Void) -> Self {
-        dispatch_async(delegate.queue) {
+        delegate.queue.addOperationWithBlock {
             let (responseObject: AnyObject?, serializationError: NSError?) = serializer(self.request, self.response, self.delegate.data)
 
             dispatch_async(queue ?? dispatch_get_main_queue()) {
@@ -742,7 +744,7 @@ public class Request {
 
     class TaskDelegate: NSObject, NSURLSessionTaskDelegate {
         let task: NSURLSessionTask
-        let queue: dispatch_queue_t
+        let queue: NSOperationQueue
         let progress: NSProgress
 
         var data: NSData? { return nil }
@@ -759,15 +761,20 @@ public class Request {
             self.task = task
             self.progress = NSProgress(totalUnitCount: 0)
             self.queue = {
-                let label: String = "com.alamofire.task-\(task.taskIdentifier)"
-                let queue = dispatch_queue_create((label as NSString).UTF8String, DISPATCH_QUEUE_SERIAL)
+                let operationQueue = NSOperationQueue()
+                operationQueue.maxConcurrentOperationCount = 1
+                operationQueue.qualityOfService = NSQualityOfService.Utility
+                operationQueue.suspended = true
 
-                dispatch_suspend(queue)
-
-                return queue
+                return operationQueue
             }()
         }
 
+        deinit {
+            queue.cancelAllOperations()
+            queue.suspended = true
+        }
+
         // MARK: NSURLSessionTaskDelegate
 
         func URLSession(session: NSURLSession, task: NSURLSessionTask, willPerformHTTPRedirection response: NSHTTPURLResponse, newRequest request: NSURLRequest, completionHandler: ((NSURLRequest!) -> Void)) {
@@ -813,7 +820,7 @@ public class Request {
                 self.error = error
             }
 
-            dispatch_resume(queue)
+            queue.suspended = false
         }
     }
 
@@ -897,7 +904,7 @@ extension Request {
         :returns: The request.
     */
     public func validate(validation: Validation) -> Self {
-        dispatch_async(delegate.queue) {
+        delegate.queue.addOperationWithBlock {
             if self.response != nil && self.delegate.error == nil {
                 if !validation(self.request, self.response!) {
                     self.delegate.error = NSError(domain: AlamofireErrorDomain, code: -1, userInfo: nil)

+ 16 - 0
Tests/ManagerTests.swift

@@ -57,4 +57,20 @@ class AlamofireManagerTestCase: XCTestCase {
         XCTAssert(request.task.state == .Suspended)
         XCTAssertNil(manager)
     }
+
+    func testReleasingManagerWithPendingCanceledRequestDeinitializesSuccessfully() {
+        var manager: Manager? = Alamofire.Manager()
+        manager!.startRequestsImmediately = false
+
+        let URL = NSURL(string: "http://httpbin.org/get")!
+        let URLRequest = NSURLRequest(URL: URL)
+
+        let request = manager!.request(URLRequest)
+        request.cancel()
+
+        manager = nil
+
+        XCTAssert(request.task.state == .Canceling)
+        XCTAssertNil(manager)
+    }
 }