Why does NSTableView crash when processing deleted rows as NSFetchedResultsControllerDelegate?

Question

I am using a fairly standard setup of NSTableView + CoreData + NSFetchedResultsController, with the relevant view controller being NSFetchedResultsControllerDelegate to receive the changes. Here are the relevant bits of code from the view controller:

func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChange anObject: Any, at indexPath: IndexPath?, for type: NSFetchedResultsChangeType, newIndexPath: IndexPath?){

    print("Change type \(type) for indexPath \(String(describing: indexPath)), newIndexPath \(String(describing: newIndexPath)). Changed object: \(anObject). FRC by this moment has \(String(describing: self.frc?.fetchedObjects?.count)) objects, tableView has \(self.tableView.numberOfRows) rows")

    switch type {
    case .insert:
        if let newIndexPath = newIndexPath {
            tableView.insertRows(at: [newIndexPath.item], withAnimation: .effectFade)
        }
    case .delete:
        if let indexPath = indexPath {
            tableView.removeRows(at: [indexPath.item], withAnimation: .effectFade)
        }
    case .update:
        if let indexPath = indexPath {
            let row = indexPath.item
            for column in 0..<tableView.numberOfColumns {
                tableView.reloadData(forRowIndexes: IndexSet(integer: row), columnIndexes: IndexSet(integer: column))
            }
        }
    case .move:
        if let indexPath = indexPath, let newIndexPath = newIndexPath {
            tableView.removeRows(at: [indexPath.item], withAnimation: .effectFade)
            tableView.insertRows(at: [newIndexPath.item], withAnimation: .effectFade)
        }
    @unknown default:
        fatalError("Unknown fetched results controller change result type")
    }
}

func controllerWillChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
    print("tableViewBeginUpdates")
    tableView.beginUpdates()
}

func controllerDidChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
    tableView.endUpdates()
    print("tableViewEndUpdates")
}

I understand that I should be able to batch all the updates this way, even if multiple rows are deleted. However, this causes a crash with multiple deletes in a row.

Here is the log output from a session, with the table initially having four rows, and all of them being deleted:

tableViewBeginUpdates
Change type NSFetchedResultsChangeType for indexPath Optional([0, 2]), newIndexPath nil. Changed object: /… correct object info …/. FRC by this moment has Optional(0) objects, tableView has 4 rows
Change type NSFetchedResultsChangeType for indexPath Optional([0, 1]), newIndexPath nil. Changed object: /… correct object info …/. FRC by this moment has Optional(0) objects, tableView has 3 rows
Change type NSFetchedResultsChangeType for indexPath Optional([0, 0]), newIndexPath nil. Changed object: /… correct object info …/. FRC by this moment has Optional(0) objects, tableView has 2 rows
Change type NSFetchedResultsChangeType for indexPath Optional([0, 3]), newIndexPath nil. Changed object: /… correct object info …/. FRC by this moment has Optional(0) objects, tableView has 1 rows

The last row causes a crash:

2019-05-06 22:01:30.968849+0300 MyApp[3517:598234] *** Terminating app due to uncaught exception 'NSTableViewException', reason: 'NSTableView error inserting/removing/moving row 3 (numberOfRows: 1).'

The first three deletes happen to be reported in the "right" order (rows with bigger indexes [row numbers] being deleted first). The last one arrives “out of order” and the other rows are seemingly already gone from NSTableView by this time.

How are objects deleted from the context in the first place: I am using the recommended best practice of having two Managed Object Contexts work against the same NSPersistentContainer, one for UI work in the main thread, and one for background/network work in background. They watch each other’s changes. This crash is triggered when the sync context receives some changes from the network, saves them, and they propagate to view context, with this method elsewhere in the app:

@objc func syncContextDidSave(note: NSNotification) {
    viewContext.perform {
        self.viewContext.mergeChanges(fromContextDidSave: note as Notification)
    }
}

Have I misunderstood how to work with the fetched results controller delegate? I thought that the beginupdates/endupdates calls make sure that the “table view model” does not change between them? What should I do to eliminate the crash?

Coben

beginupdates/endupdates groups the animations, not the removeRowss. How are the objects deleted from the context? Does anObject match indexPath? – Toady

Object deletion is triggered by another NSManagedObjectContext against the same NSPersistentCoordinator. I added some extra info in my question on this. The objects appear to be correct. The objects reported match the indexPath before the deletes. – Coben

It batches them but they are still performed in order, when the first is removed, it will change the index of the second request, and eventually it will be an index out of bounds problem. – Butene

@Digitalsa1nt thank you, that makes sense. Should I keep track of such changed indexes through processing the batched changes, and adjust indexes for each incoming change? Sounds doable, but I can't believe I'm the first one in the world having to deal with this. I'm hoping to get a readymade (hopefully proven/tested) recipe as the answer :D – Coben

@Coben what is the users experience here, do you allow them to check each row they want to remove, or do you use multi-select of the table view? You're right you're not the first to come across this. There are a few guides you can find that talk about it. – Butene

@Digitalsa1nt these deletes are triggered not by the user, but by something that arrives from the network (CloudKit specifically). The user just sees the info disappear (possibly because the items were authored by someone else who decided to remove what they had authored). – Coben

Answer 1

Updating from a fetchedResultsController is more difficult than the Apple documentation states. The code that you are sharing will cause this kinds of a bug when there is a move and an insert or a move and delete at the same time. That doesn't seem to be what is going on for your case, but this setup will fix it as well.

indexPath is the index BEFORE the deletes and inserts are applied; newIndexPath is the index AFTER the deletes and inserts are applied.

For updates you don't care where it was BEFORE the inserts and delete - only after - so use newIndexPath not indexPath. This will fix bugs that can happen when you an update and insert (or update and delete) at the same time and the cell doesn't update as you expect.

For move the delegate is saying where it moved from BEFORE the inserts and where it should be inserted AFTER the inserts and deletes. This can be challenging when you have a move and insert (or move and delete). You can't perform the move before the inserts because then the toIndex will be wrong, and you can't perform it after the inserts because then the fromIndex will be wrong. To get around this, you perform an insert and delete instead of a move.

Save all the changes from controller:didChangeObject:atIndexPath:forChangeType:newIndexPath: into three different arrays, insert, delete and update. When you get a move add an entry for it in both the insert array and in the delete array. In controllerDidChangeContent: sort the delete array descending and the insert array ascending. Then apply the changes - first delete, then insert, then update. This will fix crashes that can happens when you have a move and insert (or move and delete) at the same time.

I can't explain why you have an out of order delete. In my testing I have always seen deletes are served descending and inserts are served ascending. Nevertheless this setup will fix your issues as well, as there is a step to sort the deletes.

If you have sections then also save the sections changes in arrays, and then apply the changes in order: deletes (descending), sectionDelete (descending), sectionInserts (ascending), inserts(ascending), updates (any order). Sections can't move or be updated.

Summary:

• Have 5 arrays : sectionInserts, sectionDeletes, rowDeletes, rowInserts, and rowUpdates
• in controllerWillChangeContent clear all the arrays
• in controller:didChangeObject: add the indexPaths into the arrays (move is a delete and an insert. Update uses newIndexPath)
• in controller:didChangeSection add the section into the sectionInserts or rowDeletes array
• in controllerDidChangeContent: process them as follows:
  • sort rowDeletes descending
  • sort sectionDelete descending
  • sort sectionInserts ascending
  • sort rowInserts ascending
• then in one performBatchUpdates block apply the changes to the collectionView in order:
  • rowDeletes
  • sectionDelete
  • sectionInserts
  • rowInserts
  • rowUpdates

The ordering is very important. For example: if you have to delete row 1 and 3 and delete row 1 first, then row 3 will turn into row 2 and you will delete the wrong row! Likewise you must delete all the rows in a section before you may delete that section. Also a section needs to be inserted before rows may be added to it. When adding rows, it must be done ascending for same reason that removing rows needs to be done descending. If you are adding row 1 and row 3, and insert row 3 first it will be off by one, since the position of 3 given in by the fetchedResultsController is assuming that row 1 was already added.

Fluorene

Thank you. This seems to work. Implemented and tested it as a separate object since I can have clean unit tests for it then. Posted here: gist.github.com/jaanus/c28fa29ba24d44e1a2fff2d62e0493e6 – Coben

I have a simple project that implements this. It can be found here: github.com/jon513/FetchedResultsControllerNeverCrash – Fluorene

Great answer. And logical. This level of detail should be in Apple's documentation. – Flexion

Answer 2

Hopefully the official documentation on batch delete operations on the UITableView might help you a little here.

In the sample below, the delete operations will always run first, postponing the remove operations, but the idea is that you commit them all at the same time in between begin and end, so that the UITableView can do the heavy lifting for you.

- (IBAction)insertAndDeleteRows:(id)sender {
    // original rows: Arizona, California, Delaware, New Jersey, Washington

    [states removeObjectAtIndex:4]; // Washington
    [states removeObjectAtIndex:2]; // Delaware
    [states insertObject:@"Alaska" atIndex:0];
    [states insertObject:@"Georgia" atIndex:3];
    [states insertObject:@"Virginia" atIndex:5];

    NSArray *deleteIndexPaths = [NSArray arrayWithObjects:
                                [NSIndexPath indexPathForRow:2 inSection:0],
                                [NSIndexPath indexPathForRow:4 inSection:0],
                                nil];
    NSArray *insertIndexPaths = [NSArray arrayWithObjects:
                                [NSIndexPath indexPathForRow:0 inSection:0],
                                [NSIndexPath indexPathForRow:3 inSection:0],
                                [NSIndexPath indexPathForRow:5 inSection:0],
                                nil];
    UITableView *tv = (UITableView *)self.view;

    [tv beginUpdates];
    [tv insertRowsAtIndexPaths:insertIndexPaths withRowAnimation:UITableViewRowAnimationRight];
    [tv deleteRowsAtIndexPaths:deleteIndexPaths withRowAnimation:UITableViewRowAnimationFade];
    [tv endUpdates];

    // ending rows: Alaska, Arizona, California, Georgia, New Jersey, Virginia
}

This example removes two strings from an array (and their corresponding rows) and inserts three strings into the array (along with their corresponding rows). The next section, Ordering of Operations and Index Paths, explains particular aspects of the row (or section) insertion and deletion behavior.

The key here is that all deletion indexes are passed in at the same time in-between the begin and end update calls. If you store the indexes first, then pass them in then you hit the situation I mentioned in my comment where the indexes start to throw out of bounds exceptions.

The apple documentation can be found here, and the above sample under the heading: 'An Example of Batched Insertion and Deletion Operations'

Hope this helps point you in the right direction.

Butene

I wonder if there are any behavior differences between UITableView and NSTableView? I am dealing with NSTableView and AppKit. My code is doing the equivalent of this example (I think - the only difference that I can see, is that removeRows is called multiple times with one indexpath, not once with several indexpaths, though the multiple calles are still within beginupdates/endupdates block). I note that the method is called deleteRows for UITableView but removeRows in NSTableView - could that also indicate a behavior difference? – Coben

@Coben possibly, but looking at the AppKit documentation for NSTableView the principle seems to be the same, could you possibly show the code that ends up causing that delegate to fire in the event of a change to the data source? I'm just curious if it has an array of indexes to remove and processes them one by one until complete etc. – Butene