4

I'm working on a NSOperation subclass and I came across this very weird issue were the completion block is called twice in a row. The KVO calls seem fine but the completionBlock is still strangely called twice. Am I misunderstanding NSOperation? The documentation says that the completion block is called when isFinished becomes YES and that only happens once in my code:

- (void)main {
    @autoreleasepool {
        [self willChangeValueForKey:@"isExecuting"];
        [self willChangeValueForKey:@"isReady"];
        executing = YES;
        [self didChangeValueForKey:@"isReady"];
        [self didChangeValueForKey:@"isExecuting"];

        //start the operation
    }
}

I then simply set the completionBlock like this:

self.completionBlock = ^{
    NSLog(@"Completed");
}

When it finishes this method is called (it is called just ONCE, i double checked that)

- (void)completeOperation {
    [self willChangeValueForKey:@"isExecuting"];
    [self willChangeValueForKey:@"isFinished"];
    executing = NO;
    completed = YES;
    [self didChangeValueForKey:@"isExecuting"];
    [self didChangeValueForKey:@"isFinished"];
}

But the completionBlock is called twice and prints "Completed" twice into the console.

And here are the methods that indicate the current state:

- (BOOL)isReady {
    if (executing || cancelled || completed) {
        return NO;
    }
    return YES;
}
- (BOOL)isCancelled {
    return cancelled;
}

- (BOOL)isConcurrent {
    return YES;
}

- (BOOL)isExecuting {
    return executing;
}

- (BOOL)isFinished {
    return completed;
}

isCancelled never turns to YES in my testing code so that couldn't be the cause of it.

I really don't get why the completionBlock is called twice. Even when setting the completion block to nil from inside the completion block it is sometimes called twice which is even stranger.

tshepang
  • 12,111
  • 21
  • 91
  • 136
JonasG
  • 9,274
  • 12
  • 59
  • 88

1 Answers1

4

Not sure if this is the cause, but in my experience, there is no need to override the read-only state properties. You are responsible for checking isCancelled periodically in the main loop and if it's set bailing out of whatever you are doing, but I believe the other state flags (isReady, isFinished, isExecuting) are taken care of automatically.

How many times does it fire if you strip away the state flag handling and just do your process in -main?

EDIT: Assuming you are overriding those flags to allow concurrency, then you should read through the notes in the docs

By the looks of it, you should never need to override isReady or isCancelled and instead override -start according to the instructions in the docs.

Henri Normak
  • 4,695
  • 2
  • 23
  • 33
  • Yeah you're right! I must have misread the docs before. I am now starting a custom background thread in -start and I've remove the isReady and isCancelled overrides and now it works, the completion block is only called once :) – JonasG May 05 '13 at 11:46
  • I agree that you don't want to implement your own `isReady` or `isCancelled` methods. If you did need custom `isReady`, you have to call `super`: As docs say, "If you want to use custom conditions to determine the readiness of your operation object, you can override this method and return a value that accurately reflects the readiness of the receiver. If you do so, your custom implementation should invoke super and incorporate its return value into the readiness state of the object. Your custom implementation must also generate appropriate KVO notifications for the isReady key path." – Rob May 25 '14 at 20:01