1

I have a view which loads data via an NSOperation within an NSOperationQueue. I want to allow users to leave this view before the operation has completed. My problem is that I can't seem to consistently do this without crashing. Here is my code to start the operation:

NSOperationQueue* tmpQueue = [[NSOperationQueue alloc] init];
self.queue = tmpQueue;
[tmpQueue release]; 
SportsLoadOperation* loadOperation = [[SportsLoadOperation alloc] init];
[loadOperation addObserver:self forKeyPath:@"isFinished" options:0 context:NULL];
[self.queue addOperation:loadOperation];
[loadOperation release];    

If I leave the view while the operation is still executing, I often get this error:

[SportsViewController retain]: message sent to deallocated instance 0x38b5a0

If I try to remove the observers so that this doesn't occur, like this:

-(void)viewWillDisappear:(BOOL)animated {
    if (self.isLoadingData) {
        for (NSOperation *operation in [self.queue operations]) {
            if([operation isExecuting]) {
                [operation cancel];
                [operation removeObserver:self forKeyPath:@"isFinished"];
            }
        }
    }
    [super viewWillDisappear:animated];
}

Then I sometimes get this error:

Terminating app due to uncaught exception 'NSRangeException', reason:
'Cannot remove an observer <SportsViewController 0x661c730> for the key path "isFinished" from <SportsLoadOperation 0x66201a0> because it is not registered as an observer.'

How can I avoid these problems?

3 Answers 3

3

The 2nd error message says it all.

Have you tried to not removeObserver after [operation cancel] and see what happens then?

Have you tried to first removeObserver and only then cancel the operation?

These might help to narrow down the conditions that trigger the error. Also, you might want to add log output to the code to see when it actually executes.

And, like freespace's answer says, adding & removing observers is best done in the construction / destruction methods of the observed instances. This generally yields more stable code.

4
  • The order doesn't make a difference. I think I misread freespaces original answer - I'll try adding & removing the observer in the actual construction/destruction methods. This sounds smarter. Is it ok to reference self in dealloc, like so: [self removeObserver:self.observer forKeyPath:@"isFinished"]; [observer release], observer = nil; ? Mar 19, 2011 at 11:08
  • This was the solution for me: adding & removing observers is best done in the construction / destruction methods of the observed instances Mar 27, 2011 at 6:56
  • How is that going to work, though? The operation will try to notify the deallocated observer BEFORE the operation is deallocated.
    – Oscar
    Jan 16, 2012 at 2:47
  • Also, self.observer isn't defined. What class has that as a property? NSObject doesn't, and neither does NSOperation.
    – Oscar
    Jan 16, 2012 at 3:11
0

Looks like you have an instance of SportsLoadOperation that doesn't have SportsViewController as an observer. Are you inserting SportsLoadOperation anywhere else in your code? If this is the case, consider writing an initWithObserver method for SportsLoadOperaion that will do the observing automatically. This avoids errors caused by forgetting to set the observer on isFinished.

Also, it is probably better to do the removal of observer in dealloc then in viewWillDisappear because viewWillDisappear is called in many circumstances, e.g. when displaying a modal view controller. Thus you might be prematurely stopping your operations.

Edit

Instead of checking against [operation isExecuting] check against [operation isCancelled]. This is because [operation cancel] doesn't stop an operation - it can and will continue executing if you don't actually check for isCancelled in your main method. This means that if viewWillDisappear is called twice, you could end up attempting to call removeObserver twice on the same instance of SportsLoadOperation, with the second attempt failing.

Add some debugging output statements in the following places:

  1. when you create a SportsLoadOperation instance and insert it into the queueu
  2. when you are cancelling a SportsLoadOperation instance and removing from it observers.
5
  • Afraid not, the load operation is called from one specific "reload" function, it doesn't appear anywhere else. Fair point about prematurely stopping the operations, but doesn't help my current problem I'm afraid. Thanks for giving it a shot though. Mar 19, 2011 at 10:07
  • Are you actually stopping when cancel is sent to your SportsLoadOperation? If not, it is entirely possible for the removeObserver... code to execute twice on the same SportsLoadOperation instance because calling cancel only advise the operation to stop, doesn't make it stop executing.
    – freespace
    Mar 19, 2011 at 10:33
  • I'm not currently doing anything when cancel is called as the operation just runs a synchronous download. I could probably interrupt this but at the present time I am not. Could you explain how the removeObserver code could be executed twice? Maybe when the NSOperationQueue is deallocated? Mar 19, 2011 at 10:53
  • because calling cancel doesn't actually do anything to stop the operation, [operation isExecuting] is still true after calling it unless the operation has ended. This means that if viewDidUnload is called twice for whatever reason (and it can be if another view is covering it), then you will attempt to removeObserver twice, and it will fail the second time. Add the debug output like I suggest in the answer and see if this is the case.
    – freespace
    Mar 20, 2011 at 10:34
  • It's not safe to do anything to operations that are in a queue, because they can be deleted at any time once they're finished. Even suspending the queue won't prevent that, because already-executing operations can still finish and be deleted out from under you.
    – Oscar
    Jan 16, 2012 at 2:45
0

I ended up solving this by overriding the observed operation's addObserver and removeObserver methods, to keep track of observers so I could remove them in the [cancel] method.

All I have to do now is call the operation queue to cancel all operations before I dismiss the controller that was observing the operations.

Below, _observers is an NSMutableDictionary.

- (void)addObserver:(NSObject*)observer
     forKeyPath:(NSString*)keyPath
        options:(NSKeyValueObservingOptions)options context:(void*)context
{
    [super addObserver:observer forKeyPath:keyPath options:options context:context];
    [_observers setValue:observer forKey:keyPath];
}


- (void)removeObserver:(NSObject*)observer forKeyPath:(NSString*)keyPath
{
    [super removeObserver:observer forKeyPath:keyPath];
    [_observers removeObjectForKey:keyPath];
}


- (void)cancel
{
    [super cancel];

    for(id key in _observers)
    {
            id object = [_observers valueForKey:key];
            [super removeObserver:object forKeyPath:key];
    }

    [_observers removeAllObjects];

}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.