0

I have a shared singleton class of NSObject that I have some operation queues running in. I get a crash on this:

[super observeValueForKeyPath:keyPath ofObject:object change:change context:context];

It seems that I need to use 'removeObserver:' to prevent this from happening, but how do I properly do that on a shared object?

CODE:

-(void)synchronizeToDevice{
    queue = [NSOperationQueue new];
    queue.name = @"SynchronizeToDeviceQueue";
    //Sync Active User
    NSInvocationOperation *operationUser = [[NSInvocationOperation alloc] initWithTarget:self
                                                                                selector:@selector(downloadUserData:)
                                                                              object:[self activeUserID]];

    [queue addOperation:operationUser];

    //Sync Video Data
    NSInvocationOperation *operationVideos = [[NSInvocationOperation alloc] initWithTarget:self
                                                                            selector:@selector(downloadVideoData)
                                                                              object:nil];
    [queue addOperation:operationVideos];


    [queue addObserver:self forKeyPath:@"operations" options:0 context:NULL];
}
- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
{
    if (object == queue && [keyPath isEqualToString:@"operations"]) {
        //Synchronization Queue
        if ([queue.name isEqualToString:@"SynchronizeToDeviceQueue"] && [queue.operations count] == 0) {
            //Queue Completed
            //Notify View Synchronization Completed
            [self performSelectorOnMainThread:@selector(postNotificationDidFinishSynchronizationToDevice) withObject:nil waitUntilDone:NO];
        }
        //Video Download Queue
        if ([queue.name isEqualToString:@"VideoFileDownloadQueue"] && [queue.operations count] == 0) {
            //Notify View Video File Download Completed
            [self performSelectorOnMainThread:@selector(postNotificationDidFinishDownloadingVideo) withObject:nil waitUntilDone:NO];
        }
        //Active User Sync Queue
        if ([queue.name isEqualToString:@"SynchronizeActiveUserToDeviceQueue"] && [queue.operations count] == 0) {
            //Queue Completed
            //Notify View Synchronization Completed
            [self performSelectorOnMainThread:@selector(postNotificationDidFinishActiveUserSynchronizationToDevice) withObject:nil waitUntilDone:NO];
        }
    }
    else {
        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }
}

CRASH LOG:

2013-03-14 21:48:42.167 COMPANY[1946:1103] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: '<DataManager: 0x1c54a420>: An -observeValueForKeyPath:ofObject:change:context: message was received but not handled.
Key path: operations
Observed object: <NSOperationQueue: 0x1c5d3360>{name = 'SynchronizeActiveUserToDeviceQueue'}
Change: {
    kind = 1;
}
Context: 0x0'
*** First throw call stack:
(0x336262a3 0x3b4b197f 0x336261c5 0x33f1a56d 0x21bd1 0x33eb46b9 0x33eb4313 0x33eb3a25 0x33eb3817 0x33f2b689 0x3b8ccb97 0x3b8cf139 0x3b8cd91d 0x3b8cdac1 0x3b8fda11 0x3b8fd8a4)
libc++abi.dylib: terminate called throwing an exception
JimmyJammed
  • 9,598
  • 19
  • 79
  • 146

2 Answers2

2

In Receiving Notification of a Change of the "Key-Value Observing Programming Guide", an example implementation of observeValueForKeyPath is given, with the comment:

Be sure to call the superclass's implementation if it implements it. NSObject does not implement the method.

You said that your class is a subclass of NSObject, therefore you should not call [super observeValueForKeyPath:...].

Another problem would occur if you call synchronizeToDevice more than once on the same shared instance. In that case you create a new queue and register an observer for that. But the observer for the old queue is not removed.

As a consequence, observeValueForKeyPath might be called for a "old queue" and the check if (object == queue) fails, resulting in the unwanted call to super.

So if synchronizeToDevice can be called multiple times, you should remove the old observer first.

Martin R
  • 529,903
  • 94
  • 1,240
  • 1,382
  • 1
    While true and helpful, I don't think this is the actual problem. What the error is telling us is that the KVO callback was called when no one was expecting it (no one handled it). Just removing the call to `super` would mask that problem, but the problem would remain. The call to `super` essentially is acting like an `NSAssert(I_SHOULD_HAVE_HANDLED_THAT)`. – Rob Napier Mar 20 '13 at 21:58
  • @RobNapier: Yes, that sounds reasonable. – Martin R Mar 20 '13 at 22:03
  • @RobNapier: I had not noticed that you posted an answer while I was updating mine. – Martin R Mar 20 '13 at 22:11
2

I suspect your call to synchronizeToDevice is being called more than once. If so, you are continuing to observe the old queue, and also some new queue. When observeValueForKeyPath:... fires, it is likely passing you the old queue, which you then ignore, call super, which throws the exception because you didn't handle the observation you asked for.

Your real problem here is that you're not using accessors. That would have made this much clearer. For instance, this is how you would implement setQueue:

-(void)setQueue:(NSOperationQueue *)queue {
  if (_queue) {
    [_queue removeObserver:self forKeyPath:@"operations"];
  }

  _queue = queue;

  if (_queue) {
    [_queue addObserver:self forKeyPath:@"operations" options:0 context:NULL];
  }
}

Now when you call self.queue = [NSOperationQueue new];, everything automatically works. You stop observing the old queue and start observing the new queue. If you call self.queue = nil it automatically unregisters for you.

You still need to make sure to unregister in dealloc:

- (void)dealloc {
  if (_queue) {
    [_queue removeObserver:self forKeyPath:@"operations"];
  }
}
Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • So you are correct, each time the popover is opened it calls synchronizeToDevice. So rapid open/close of the popover eventually causes the crash. However, I added the custom setQueue method you provided, but it still crashes. I set breakpoints to ensure it is called and it is so not sure why it's still crashing. Any suggestions? – JimmyJammed Mar 21 '13 at 03:41