4

I have an NSOperation subclass that runs async operations from a UITableView.

I override the correct start and finish methods like this:

- (void)start
{
    [self willChangeValueForKey:@"isExecuting"];
    self.isExecuting = YES;
    [self didChangeValueForKey:@"isExecuting"];

    if (self.isCancelled)
    {
        [self finish];
        return;
    }
}
- (void)finish
{
       if (!_isExecuting)
        {
            [self willChangeValueForKey:@"isExecuting"];
            _isExecuting = YES;
            [self didChangeValueForKey:@"isExecuting"];
        }

        [self willChangeValueForKey:@"isExecuting"];
        [self willChangeValueForKey:@"isFinished"];

        _isExecuting = NO;
        _isFinished = YES;

        [self didChangeValueForKey:@"isExecuting"];
        [self didChangeValueForKey:@"isFinished"];
}

The problem I have, it if I scroll down the table and delete a row, this calls the cancel method on the operation, however as the operations gradually complete and it reaches further down the table, it crashes with an EXC_BAD_ACCESS error on the line [self didChangeValueForKey:@"isFinished"];

The code is pretty complex to paste it all here, but what i'd like to know is how can I track down what object is causing the KVO message to crash?

If I enable zombie objects in the debugger, it simply doesn't crash at all with no warnings which doesn't help.

If I wrap the KVO methods in try/catch it is never caught and still crashes.

I have tried overriding the KVO methods in my NSOperation subclass, but they are never called:

- (void)addObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context
{
    NSLog(@"%s - %@", __PRETTY_FUNCTION__, observer);
    [super addObserver:observer forKeyPath:keyPath options:options context:context];
}

Is it possible to see who the observers are?

Darren
  • 10,182
  • 20
  • 95
  • 162

3 Answers3

3

Comments and ideas:

  1. In start shouldn't self.isExecuting = YES be _isExecuting = YES;?
  2. Add a property called identifierof type NSString, and set it for each op.
  3. Add a dealloc method and log the identifier.
  4. In finish test for isCancelled and return immediately if so.

Another thought is to doubly retain the operations - put them in a NSDictionary with the identifier as the key, and see if anything changes.

David H
  • 40,852
  • 12
  • 92
  • 138
  • 1, thanks changed. 2&3 I used `indexPath` and deleted indexPath 4-14 which cancelled and dealloc'd the `NSOperation` immediately. Just after the operation queue finished indexPath 4-9 it crashed. From my understanding of the Apple docs, even cancelled operations get started then finish. So it must be trying to start 4-14 but as it's been dealloc'd I don't see how it can crash on the finish method. – Darren Jan 27 '16 at 13:33
  • 1
    Its been a long time since I used NSOperations - I switched to GCD as soon as it came out. Why didn't you try 4? Its possible you trying to change properties after cancelled is causing problems... – David H Jan 27 '16 at 13:59
  • 1
    I think I have fixed it, and it's related to 4. In my cancel method I was manually calling the finished method. I'm sure I read somewhere to do this as the cancel method is simply just flipping `isCancelled` bool. Removing that seems to have stopped the crash – Darren Jan 27 '16 at 14:16
2

I met with the same problem. It occurs because you set the value for the isFinished variable before the-start method is called.

To solve this problem you need to set the flag isCancelled = YES (and only) in the-cancel method. And in the method -start to do a check on the state of cancellation.

If the operation was canceled before it started, then you should already set these values:

    self.executing = YES;
    self.executing = NO;
    self.finished  = YES;

All code:

-(void)start
{
   if ((self.ready == YES) && (self.executing == NO) && ((self.finished == NO)) // if Ready To Start State
  {
    self.ready = NO; self.executing = YES; self.finished = NO; // Setting  Working State

    [self receiveItemAtURL:self.url params:self.params
           timeStoreExpire:self.storageTime
                  progress:self.operationProgressBlock
                completion:self.operationCompletionBlock];

  }else if ((self.ready == NO) && (self.cancelled == YES){ // If Cancelled State
    self.executing = YES;
    self.executing = NO;
    self.finished  = YES;
   }
}


- (void) cancel
 {
    if ((self.ready == YES) && (self.executing == NO) && ((self.finished == NO))      { // If Working State
    self.ready = NO; self.cancelled = YES; // Setting Cancelled State
     }
    else if ((self.ready == NO) && (self.executing == YES) && (self.finished == NO) || // If Working State
         (self.ready == YES) && (self.executing == NO) && (self.finished == NO))   // If Suspend State
   {
    self.state = RXCOCancelled;
    self.executing = YES;
    self.executing = NO;
    self.finished  = YES;
   }
 }
I.U.
  • 319
  • 1
  • 2
  • 5
0

I ran into the same issue on MacOS and realised it is caused by setting isFinished on an operation that has been cancelled, which will then remove it from the queue immediately, but out of turn.

Given there is a check for isCancelled inside the start function, there is no need to set isFinished when you cancel.

Instead, your start will take care of that and the queue will remove accordingly.

You should also check for isExecuting

override func start() {
    isExecuting = true
    isFinished = false
    guard !isCancelled else {
        finish()
        return
    }

    if let willStart = exportProgress.willStartQueuedOperation {
        willStart()
    }
}

func finish() {
    isExecuting = false
    isFinished = true
}

override func cancel() {
    super.cancel()
    if isExecuting {
        finish()
    }
}
Rob Blankson
  • 1
  • 1