4
votes

I have an NSOperation with an NSOperationQueue that has a bunch of child operations, some queued up.

I had a problem where even after calling cancelAllOperations on the queue my main method was hanging on waitUntilAllOperationsAreFinished.

Then when I set the complete flag I use for isFinished upon cancellation it no longer gets backed up in a cancelled queue.

- (BOOL)isFinished
{
    return complete;
}

- (void)cancel
{
    cancelled = YES;
    complete = YES;
    [_childOperationQueue cancelAllOperations];
}

Is this the correct behaviour, a cancelled operation should be technically finished? It seems like NSOperation needs the isFinished to be set to true before it will remove it, in thought this might allow it to 'clean up', but I don't know what the protocol is here and google didn't reveal much.

2
Are you sure your operation is actually cancelled? Telling it to cancel does not immediately cancel it, it just tells the block it should cancel itself. You need to actually check self.cancelled and abort your operation. You should not be overriding isFinished or cancel — the default behaviour for both should work. - Abhi Beckert
@AbhiBeckert If writing a non-concurrent operation, you are correct. But when writing concurrent operations, you always override isFinished method (either explicitly, or with automatically synthesized getter). Also, re cancel, while I agree that you don't generally override that, just check isCancelled periodically, sometimes you cannot do that. For example, if you initiate a block-based NSURLSessionTask, there is no where you can check isCancelled, so you will override cancel method in that scenario. - Rob

2 Answers

4
votes

Canceling the operation just sets isCancelled to return YES. This means in your NSOperation block you can check for isCancelled and prevent unnecessarily doing any work (you need to implement this logic yourself).

Your main thread needs to wait for all the operations on the queue, but if your NSOperation block checks isCancelled before doing anything you should quickly get through all the queued operations and the wait shouldn't be long.

3
votes

If you are writing a concurrent operation (i.e. isConcurrent method returns YES), I have two observations:

  1. When you change the state of your operation to be finished, you have to manually perform the appropriate isFinished KVN. Thus, before you change your state variable, call willChangeValueForKey, and after you change it, call didChangeValueForKey.

    There are many ways to achieve this, but I define my finished and executing properties like so:

    @property (nonatomic, readwrite, getter = isFinished)  BOOL finished;
    @property (nonatomic, readwrite, getter = isExecuting) BOOL executing;
    

    And I use those synthesized getters, but write setters that do the appropriate KVN:

    @synthesize finished  = _finished;
    @synthesize executing = _executing;
    
    - (void)setExecuting:(BOOL)executing
    {
        if (_executing != executing) {
            [self willChangeValueForKey:@"isExecuting"];
            _executing = executing;
            [self didChangeValueForKey:@"isExecuting"];
        }
    }
    
    - (void)setFinished:(BOOL)finished
    {
        if (_finished != finished) {
            [self willChangeValueForKey:@"isFinished"];
            _finished = finished;
            [self didChangeValueForKey:@"isFinished"];
        }
    }
    

    Then, when I want to complete the operation, I call a method like so:

    - (void)completeOperation
    {
        self.executing = NO;
        self.finished = YES;
    }
    

    That results in the KVN for isExecuting and isFinished (as well as ensuring that my synthesized getters return the appropriate values, too). If you don't do this, your operations may never finish.

  2. In terms of canceling the operation, there are two approaches:

    • You can periodically check [self isCancelled] (a very common pattern if you have a loop or some method which is being called repeatedly, e.g. a didReceiveData in a delegate based connection/session), and if so, do whatever clean-up you need and then also complete the operation (the above completeOperation method).

    • If you don't have any place you can periodically check the cancellation status, you want to override the cancel method, which does whatever cleanup you need, calls [super cancel] but also ensures that the operation finishes, too (e.g. call completeOperation if the operation is executing).

    I also have start check the cancellation status:

    - (void)start
    {
        if ([self isCancelled]) {
            self.finished = YES;
            return;
        }
    
        self.executing = YES;
    
        // start my operation
    }
    

Beyond the above, it's hard to counsel you further without knowing a bit more about what's in the heart of the operation (NSURLConnection? NSURLSession? something else?).

For more information, see the Configuring Operations for Concurrent Execution in the Concurrency Programming Guide: Operation Queues.