Cancelling an Alamofire Request Wrapped In NSOperation Causes Multiple KVO?

Question

My Xcode version: 6.3.2
Alamofire version: 1.2.2 (installed via Cocoapods)

In order to set maxConcurrentOperationCount to limit the concurrent operation number in a NSOperationQueue, I wrap my Alamofire download request in a NSOperation just like Rob suggested.

The basic subclass of NSOperation like this:

class ConcurrentOperation : NSOperation {

    override var concurrent: Bool {
        return true
    }

    override var asynchronous: Bool {
        return true
    }

    private var _executing: Bool = false
    override var executing: Bool {
        get {
            return _executing
        }
        set {
            if (_executing != newValue) {
                self.willChangeValueForKey("isExecuting")
                _executing = newValue
                self.didChangeValueForKey("isExecuting")
            }
        }
    }

    private var _finished: Bool = false;
    override var finished: Bool {
        get {
            return _finished
        }
        set {
            if (_finished != newValue) {
                self.willChangeValueForKey("isFinished")
                _finished = newValue
                self.didChangeValueForKey("isFinished")
            }
        }
    }

    /// Complete the operation
    ///
    /// This will result in the appropriate KVN of isFinished and isExecuting

    func completeOperation() {
        executing = false
        finished  = true
    }

    override func start() {
        if (cancelled) {
            finished = true
            return
        }

        executing = true

        main()
    }
}

And my subclass wrapping an Alamofire download request like this:

class DownloadImageOperation : ConcurrentOperation {
    let URLString: String
    let downloadImageCompletionHandler: (responseObject: AnyObject?, error: NSError?) -> ()
    weak var request: Alamofire.Request?

    init(URLString: String, downloadImageCompletionHandler: (responseObject: AnyObject?, error: NSError?) -> ()) {
        self.URLString = URLString
        self.downloadImageCompletionHandler = downloadImageCompletionHandler
        super.init()
    }

    override func main() {
        let destination = Alamofire.Request.suggestedDownloadDestination(directory: .DocumentDirectory, domain: .UserDomainMask)
        request = Alamofire.download(.GET, URLString, destination).response { (request, response, responseObject, error) in
            if self.cancelled {
                println("Alamofire.download cancelled while downlading. Not proceed.")
            } else {
                self.downloadImageCompletionHandler(responseObject: responseObject, error: error)
            }
            self.completeOperation()
        }
    }

    override func cancel() {
        request?.cancel()
        super.cancel()
    }
}

It overrides cancel() and tries to cancel the Alamofire request when the NSOperation is cancelled.

I used a KVO observer to watch the completion of NSOperationQueue.

private var testAlamofireContext = 0

class TestAlamofireObserver: NSObject {
    var queue = NSOperationQueue()

    init(delegate: ImageDownloadDelegate) {
        super.init()
        queue.addObserver(self, forKeyPath: "operations", options: .New, context: &testAlamofireContext)
    }

    deinit {
        queue.removeObserver(self, forKeyPath: "operations", context: &testAlamofireContext)
    }

    override func observeValueForKeyPath(keyPath: String, ofObject object: AnyObject, change: [NSObject: AnyObject], context: UnsafeMutablePointer<Void>) {
        if context == &testAlamofireContext {
            if self.queue.operations.count == 0 {
                println("Image Download Complete queue. keyPath: \(keyPath); object: \(object); context: \(context)")
            }
        } else {
            super.observeValueForKeyPath(keyPath, ofObject: object, change: change, context: context)
        }
    }
}

I started a list of downloading like this:

func downloadImages() {
    let imgLinks = [
        "https://farm4.staticflickr.com/3925/18769503068_1fc09427ec_k.jpg",
        "https://farm1.staticflickr.com/338/18933828356_4f57420df7_k.jpg",
        "https://farm4.staticflickr.com/3776/18945113685_ccec89d67a_o.jpg",
        "https://farm1.staticflickr.com/366/18333992053_725f21166e_k.jpg",
        "https://farm4.staticflickr.com/3777/18962702032_086453ee7a_k.jpg",
        "https://farm1.staticflickr.com/373/18930501406_4753ac021a_k.jpg",
        "https://farm1.staticflickr.com/283/18772907409_56ffbe573b_k.jpg",
        "https://farm1.staticflickr.com/314/18940901785_b0564b1c9b_o.jpg",
        "https://farm1.staticflickr.com/502/18949263495_88d75d2d2f_k.jpg",
        "https://farm4.staticflickr.com/3912/18938184302_6e0ca9ad31_k.jpg",
        "https://farm1.staticflickr.com/356/18957923475_3dc9df7634_k.jpg",
        "https://farm1.staticflickr.com/378/18925014986_e87feca9c7_o.jpg",
        "https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg",
        "https://farm1.staticflickr.com/303/18920711216_4684ff4295_k.jpg",
        "https://farm1.staticflickr.com/558/18935058546_fc10d10855_k.jpg",
        "https://farm1.staticflickr.com/384/18955290345_fb93d17828_o.jpg",
        "https://farm1.staticflickr.com/366/18333992053_725f21166e_k.jpg",
        "https://farm4.staticflickr.com/3777/18962702032_086453ee7a_k.jpg",
        "https://farm1.staticflickr.com/373/18930501406_4753ac021a_k.jpg",
        "https://farm1.staticflickr.com/283/18772907409_56ffbe573b_k.jpg",
        "https://farm1.staticflickr.com/314/18940901785_b0564b1c9b_o.jpg",
        "https://farm1.staticflickr.com/502/18949263495_88d75d2d2f_k.jpg",
        "https://farm4.staticflickr.com/3912/18938184302_6e0ca9ad31_k.jpg",
        "https://farm1.staticflickr.com/356/18957923475_3dc9df7634_k.jpg",
        "https://farm1.staticflickr.com/378/18925014986_e87feca9c7_o.jpg",
        "https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg",
        "https://farm1.staticflickr.com/303/18920711216_4684ff4295_k.jpg",
        "https://farm1.staticflickr.com/558/18935058546_fc10d10855_k.jpg",
        "https://farm1.staticflickr.com/366/18333992053_725f21166e_k.jpg",
        "https://farm4.staticflickr.com/3777/18962702032_086453ee7a_k.jpg",
        "https://farm1.staticflickr.com/373/18930501406_4753ac021a_k.jpg",
        "https://farm1.staticflickr.com/283/18772907409_56ffbe573b_k.jpg",
        "https://farm1.staticflickr.com/314/18940901785_b0564b1c9b_o.jpg",
        "https://farm1.staticflickr.com/502/18949263495_88d75d2d2f_k.jpg",
        "https://farm4.staticflickr.com/3912/18938184302_6e0ca9ad31_k.jpg",
        "https://farm1.staticflickr.com/356/18957923475_3dc9df7634_k.jpg",
        "https://farm1.staticflickr.com/378/18925014986_e87feca9c7_o.jpg",
        "https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg",
        "https://farm1.staticflickr.com/303/18920711216_4684ff4295_k.jpg",
        "https://farm1.staticflickr.com/558/18935058546_fc10d10855_k.jpg",
        "https://farm4.staticflickr.com/3777/18962702032_086453ee7a_k.jpg",
        "https://farm1.staticflickr.com/373/18930501406_4753ac021a_k.jpg",
        "https://farm1.staticflickr.com/283/18772907409_56ffbe573b_k.jpg",
        "https://farm1.staticflickr.com/314/18940901785_b0564b1c9b_o.jpg",
        "https://farm1.staticflickr.com/502/18949263495_88d75d2d2f_k.jpg",
        "https://farm4.staticflickr.com/3912/18938184302_6e0ca9ad31_k.jpg",
        "https://farm1.staticflickr.com/356/18957923475_3dc9df7634_k.jpg",
        "https://farm1.staticflickr.com/378/18925014986_e87feca9c7_o.jpg",
        "https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg",
        "https://farm1.staticflickr.com/303/18920711216_4684ff4295_k.jpg",
        "https://farm1.staticflickr.com/558/18935058546_fc10d10855_k.jpg",
        "https://farm1.staticflickr.com/266/18956724112_6e61a743a5_k.jpg"
    ]

    var testAlamofireObserver = TestAlamofireObserver()
    testAlamofireObserver!.queue.maxConcurrentOperationCount = 5

    for imgLink in imgLinks {
        let operation = DownloadImageOperation(URLString: imgLink) {
            (responseObject, error) in

            if responseObject == nil {
                // handle error here
                println("failed: \(error)")
            } else {
                println("\(responseObject?.absoluteString) downloaded.")
            }
        }
        testAlamofireObserver!.queue.addOperation(operation)
    }
}

If the queue completed without receiving any cancellation, the log outputs should be:

2015-06-22 17:11:04.206 RSS Wallpaper Switchr[46250:714702] Optional(Optional("https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg")) downloaded.
...
...
...
2015-06-22 17:11:56.979 RSS Wallpaper Switchr[46250:714702] Optional(Optional("https://farm1.staticflickr.com/461/18949863812_ddf700bd03_o.jpg")) downloaded.
2015-06-22 17:11:56.979 RSS Wallpaper Switchr[46250:714702] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x6180002354a0>{name = 'NSOperationQueue 0x6180002354a0'}; context: 0x000000010007eb70

If the queue receives cancelAllOperations(), the log outputs should be:

2015-06-22 17:16:29.691 RSS Wallpaper Switchr[46467:720630] Optional(Optional("https://farm1.staticflickr.com/366/18333992053_725f21166e_k.jpg")) downloaded.
2015-06-22 17:16:32.632 RSS Wallpaper Switchr[46467:720630] Alamofire.download cancelled while downlading. Not proceed.
...
...
2015-06-22 17:16:32.642 RSS Wallpaper Switchr[46467:720630] Alamofire.download cancelled while downlading. Not proceed.
2015-06-22 17:16:32.643 RSS Wallpaper Switchr[46467:720630] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x600000024c20>{name = 'NSOperationQueue 0x600000024c20'}; context: 0x000000010007eb70

However, if I changed maxConcurrentOperationCount to non-default value as above, and the queue receives cancelAllOperations(), the log became:

2015-06-22 17:17:56.427 RSS Wallpaper Switchr[46606:722523] Optional(Optional("https://farm4.staticflickr.com/3777/18962702032_086453ee7a_k.jpg")) downloaded.
2015-06-22 17:17:58.675 RSS Wallpaper Switchr[46606:722523] Alamofire.download cancelled while downlading. Not proceed.
...
...
2015-06-22 17:17:58.677 RSS Wallpaper Switchr[46606:722523] Alamofire.download cancelled while downlading. Not proceed.
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722720] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722560] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722574] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722719] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722721] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70
2015-06-22 17:17:58.678 RSS Wallpaper Switchr[46606:722572] Image Download Complete queue. keyPath: operations; object: <NSOperationQueue: 0x608000424ee0>{name = 'NSOperationQueue 0x608000424ee0'}; context: 0x000000010007eb70

The KVO observeValueForKeyPath was executed from multiple different threads. The number of threads may be variable. This will result the completion function of KVO to be executed several times. And this condition does not happen if I do not change the default of maxConcurrentOperationCount or do not request?.cancel() for Alamofire.Request.

Why do I care about more than one executions of the KVO completion function? My purpose is to start a download queue, when enough downloads complete, cancel the remaining operations, even non-started or in downloading, and then do something for the downloads. The completion function is supposed to execute only once, and two factors (1) change the default of maxConcurrentOperationCount (2) do not request?.cancel() for Alamofire.Request may be related to it. I'd like to know why and how to correct this.

The AlamoFire [request's `cancel()` method](https://github.com/Alamofire/Alamofire/blob/922c1ca3580b7adc9235bede1e99580e02ad3e9f/Source/Request.swift#L205) looks pretty straightforward. I don't think the issue is there. — Aaron Brager, Jun 20 '15 at 16:14
Thanks Rob. I replaced `println` with `NSLog` and then the log outputs were more sensible. However, the code I supplied might miss a key point. (please see my edit in the question above.) I found if I set `maxConcurrentOperationCount` for the `NSOperationQueue`, the cancellation will cause strange behavior. However, forget about the non-specific `NSZombie` objects and over-released non-specific objects as previously mentioned. It may be due to other codes not shown above. It was resolved. — tsaiid, Jun 22 '15 at 08:51
I have revised the question. I think `cancel()` works only on the `NSOperation` wrapping Alamofire. It should not change the behavior of `NSOperationQueue`. Therefore, the behavior of KVO observer for `NSOperationQueue` should not be changed. — tsaiid, Jun 23 '15 at 00:34

Rob · Accepted Answer · 2015-06-23T16:16:39.330

I don't find the multiple KVN behavior you describe as that surprising. There's nothing in the documentation that says that when it cancels all operations, that a single KVN on operations will result. In fact, one might safely infer that the behavior you describe should be expected (because it doesn't preemptively kill all of those worker threads, but rather sends a cancel message to each, and each operation is responsible for responding to that in its own time; and I wouldn't expect operations to be updated until the operation finally actually finishes).

Personally, I would advise retiring this observer pattern entirely. Your code should not be contingent upon whether NSOperationQueue removes all of operations at once or not. I would instead suggest that you instead rely upon your existing downloadImageCompletionHandler closure, calling it whether the request completed or not. Just have the closure look at the error object to figure out whether it was canceled or whether it failed for some other reason.

If your intent is to know when all of these operations are done, I wouldn't rely on operations KVN. Instead, I might create a completion operation, dependent upon all of those other requests:

let completionOperation = NSBlockOperation() {                    // create completion operation
    // do whatever you want here
}

for imgLink in imgLinks {
    let operation = DownloadImageOperation(URLString: imgLink) { responseObject, error in
        if error != nil {
            if error!.code == NSURLErrorCancelled && error!.domain == NSURLErrorDomain {
                println("everything OK, just canceled")
            } else {
                println("error=\(error)")
            }
        }
        if responseObject != nil {
            println("\(responseObject?.absoluteString) downloaded.")
        }
    }

    completionOperation.addDependency(operation)                 // add dependency

    testAlamofireObserver!.queue.addOperation(operation)
}

NSOperationQueue.mainQueue().addOperation(completionOperation)   // schedule completion operation on some other queue (so that when I cancel everything on that other queue, I don't cancel this, too)

Thanks for your suggestion. I'll think about using `downloadImageCompletionHandler` closure. However, I still do not understand how and why `maxConcurrentOperationCount` and `request?.cancel()` interact with each other. My current solution is let `maxConcurrentOperationCount` to be default `NSOperationQueueDefaultMaxConcurrentOperationCount`, which is [recommended by Apple](https://goo.gl/umHFNM) and let the system set the maximum number of operations based on system conditions. — tsaiid, Jun 23 '15 at 14:59
I would not advise `NSOperationQueueDefaultMaxConcurrentOperationCount` for networking operations. If you issue too many requests, you risk having the latter ones timing out. And it certainly is wrong to do that because you chose to observe `operations` and it didn't behave the way you expected. Rather than relying upon `operations` KVN, I'd use completion operation. See code snippet I added to answer. — Rob, Jun 23 '15 at 16:17

Cancelling an Alamofire Request Wrapped In NSOperation Causes Multiple KVO?

1 Answers1

Linked