Search code examples
iosswiftnsfetchedresultscontroller

NSFetchedResultsController 'didChange' using new CollectionDiffing causes strange change operations that lead to a crash


Example Project demonstrating the crash: https://github.com/d3mueller/TestProjectFetchedResultsController


I have a simple CoreData setup with two entities:

  • City, has a to-many relationship incidentRoads to Road (set to Cascade when city is deleted)
  • Road, has a to-many relationship cities to City

As an example, I've added two Cities and one Road that "connects" both cities. In my ViewController I have set up two NSFetchedResultsController that are responsible for keeping the two arrays var cities: [City] and var roads: [Road] in said view controller up-to-date.

When I go ahead and delete one of the two cities while the NSFetchedResultsController observe everything, the app crashed due to an Index out of range error while trying to update the arrays in a didChange method of the fetched results controller's delegate:

Fatal error: Index out of range

I'm using a new API of the fetched results controller's delegate to track changes via a CollectionDifference:

func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChangeContentWith diff: CollectionDifference<NSManagedObjectID>)

And after the deletion of one of the cities, that method is called multiple times with strange insertions and deletions (I don't think it would be helpful to post these here without context of the debugger. I've added an example project below).

The method is being called with a diff that looks really strange:

remove(offset: 1, element: 0xcb6b486b0bd166fe <x-coredata://E2AA7DB9-7A1F-4130-9AE4-A9D0DC695159/City/p2>, associatedWith: Optional(0))

remove(offset: 0, element: 0xcb6b486b0bdd66fe <x-coredata://E2AA7DB9-7A1F-4130-9AE4-A9D0DC695159/City/p1>, associatedWith: nil)

insert(offset: 0, element: 0xcb6b486b0bd166fe <x-coredata://E2AA7DB9-7A1F-4130-9AE4-A9D0DC695159/City/p2>, associatedWith: Optional(1))

What I expected the diff to look like is just one remove of the first element in the array (the first city that is deleted). But this doesn't make any sense to me. I don't even understand what these actually try to tell me. The first remove is associated with itself? And the last insert is associated with the second remove. I don't get it. I think the problem lies here. Any ideas?

My view controller looks like this (I commented where the crash occurs):

class ViewController: UIViewController, NSFetchedResultsControllerDelegate {

    public var cities: [City] = []
    private lazy var citiesResultsController: NSFetchedResultsController<City> = {
        let request: NSFetchRequest<City> = City.fetchRequest()
        request.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: true)]

        let controller = NSFetchedResultsController(fetchRequest: request, managedObjectContext: AppDelegate.managedObjectContext, sectionNameKeyPath: nil, cacheName: nil)

        controller.delegate = self
        return controller
    }()

    public var roads: [Road] = []
    private lazy var roadsResultsController: NSFetchedResultsController<Road> = {
        let request: NSFetchRequest<Road> = Road.fetchRequest()
        request.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: true)]

        let controller = NSFetchedResultsController(fetchRequest: request, managedObjectContext: AppDelegate.managedObjectContext, sectionNameKeyPath: nil, cacheName: nil)

        controller.delegate = self
        return controller
    }()

    func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChangeContentWith diff: CollectionDifference<NSManagedObjectID>) {
        if controller === citiesResultsController {
            for change in diff {
                switch change {
                case .insert(offset: let newPosition, element: _, associatedWith: let oldPosition):

                    if let oldPosition = oldPosition {
                        // was moved
// HERE IT CRASHES 
                        let city = cities.remove(at: oldPosition)
                        cities.insert(city, at: newPosition)
                    } else {
                        // was inserted
                        let city = citiesResultsController.object(at: IndexPath(item: newPosition, section: 0))
                        cities.insert(city, at: newPosition)
                    }
                case .remove(offset: let position, element: _, associatedWith: let associatedWith):
                    if associatedWith == nil {
                        _ = cities.remove(at: position)
                    }
                }
            }
        } else {
            for change in diff {
                switch change {
                case .insert(offset: let newPosition, element: _, associatedWith: let oldPosition):
                    if let oldPosition = oldPosition {
                        // was moved
                        let road = roads.remove(at: oldPosition)
                        roads.insert(road, at: newPosition)
                    } else {
                        // was inserted
                        let road = roadsResultsController.object(at: IndexPath(item: newPosition, section: 0))
                        roads.insert(road, at: newPosition)
                    }
                case .remove(offset: let position, element: _, associatedWith: let associatedWith):
                    if associatedWith == nil {
                        _ = roads.remove(at: position)
                    }
                }
            }
        }
    }

    override func viewDidLoad() {
        super.viewDidLoad()

        try? citiesResultsController.performFetch()
        cities = citiesResultsController.fetchedObjects ?? []

        try? roadsResultsController.performFetch()
        roads = roadsResultsController.fetchedObjects ?? []

        DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(5)) {
            print("Now")
            AppDelegate.managedObjectContext.delete(AppDelegate.cityA)
            try! AppDelegate.managedObjectContext.save()
        }
    }
}

And my appDelegate (where I set up the objects and stuff to test everything):

//
//  AppDelegate.swift
//  TestProject
//
//  Created by Dennis Müller on 23.09.19.
//  Copyright © 2019 Dennis Müller. All rights reserved.
//

import UIKit
import CoreData

@UIApplicationMain
class AppDelegate: UIResponder, UIApplicationDelegate {

    public static var managedObjectContext: NSManagedObjectContext {
        return (UIApplication.shared.delegate as! AppDelegate).persistentContainer.viewContext
    }

    public static var cityA: City!

    func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
        // Override point for customization after application launch.

        if !UserDefaults.standard.bool(forKey: "setup") {
            UserDefaults.standard.set(true, forKey: "setup")

            let cityA = City(context: persistentContainer.viewContext)
            cityA.name = "cityA"
            cityA.creationDate = Date()

            let cityB = City(context: persistentContainer.viewContext)
            cityB.name = "cityB"
            cityB.creationDate = Date()

            let road = Road(context: persistentContainer.viewContext)
            road.creationDate = Date()
            road.addToCities(cityA)
            road.addToCities(cityB)

            saveContext()

        }

        let request: NSFetchRequest<City> = City.fetchRequest()
        request.predicate = NSPredicate(format: "name == %@", "cityA")
        AppDelegate.cityA = try! persistentContainer.viewContext.fetch(request).first!

        return true
    }

    // MARK: UISceneSession Lifecycle

    func application(_ application: UIApplication, configurationForConnecting connectingSceneSession: UISceneSession, options: UIScene.ConnectionOptions) -> UISceneConfiguration {
        // Called when a new scene session is being created.
        // Use this method to select a configuration to create the new scene with.
        return UISceneConfiguration(name: "Default Configuration", sessionRole: connectingSceneSession.role)
    }

    func application(_ application: UIApplication, didDiscardSceneSessions sceneSessions: Set<UISceneSession>) {
        // Called when the user discards a scene session.
        // If any sessions were discarded while the application was not running, this will be called shortly after application:didFinishLaunchingWithOptions.
        // Use this method to release any resources that were specific to the discarded scenes, as they will not return.
    }

    // MARK: - Core Data stack

    lazy var persistentContainer: NSPersistentContainer = {
        /*
         The persistent container for the application. This implementation
         creates and returns a container, having loaded the store for the
         application to it. This property is optional since there are legitimate
         error conditions that could cause the creation of the store to fail.
        */
        let container = NSPersistentContainer(name: "TestProject")
        container.loadPersistentStores(completionHandler: { (storeDescription, error) in
            if let error = error as NSError? {
                // Replace this implementation with code to handle the error appropriately.
                // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.

                /*
                 Typical reasons for an error here include:
                 * The parent directory does not exist, cannot be created, or disallows writing.
                 * The persistent store is not accessible, due to permissions or data protection when the device is locked.
                 * The device is out of space.
                 * The store could not be migrated to the current model version.
                 Check the error message to determine what the actual problem was.
                 */
                fatalError("Unresolved error \(error), \(error.userInfo)")
            }
        })
        return container
    }()

    // MARK: - Core Data Saving support

    func saveContext () {
        let context = persistentContainer.viewContext
        if context.hasChanges {
            do {
                try context.save()
            } catch {
                // Replace this implementation with code to handle the error appropriately.
                // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.
                let nserror = error as NSError
                fatalError("Unresolved error \(nserror), \(nserror.userInfo)")
            }
        }
    }

}

Sorry if this is a bit confusing, I don't know how to properly explain the problem (I don't even know what the problem is in the first place). Let me know if you need more information.

Thank you very much for your help.


Solution

  • Okay, so turns out I'm just plain stupid. When iterating through the changes in the diff, I incorrectly assumed that the associatedWith of the .insert case is the old index of the element that is supposed to move:

    ...
                case .insert(offset: let newPosition, element: _, associatedWith: let oldPosition):
    
                        // I called the associatedWith 'oldPosition'
                        if let oldPosition = oldPosition {
                            // was moved
    // HERE IT CRASHES 
                            let city = cities.remove(at: oldPosition)
                            cities.insert(city, at: newPosition)
                        } else {
                            // was inserted
                            let city = citiesResultsController.object(at: IndexPath(item: newPosition, section: 0))
                            cities.insert(city, at: newPosition)
                        }
                ...
    

    That is causing the crash because the associatedWith is actually the index of the .remove change that's connected to it to indicate it's actually a move instead of an insert.

    So I replaced this:

    case .insert(offset: let newPosition, element: _, associatedWith: let oldPosition):
    ...
       let city = cities.remove(at: oldPosition)
    ...
    

    with this:

    case .insert(offset: let newPosition, element: let objectID, associatedWith: let associatedWith):
    ...
        let city = cities.first(where: {$0.objectID == objectID})!
    ....
    

    This is not optimal, it adds a linear time complexity to the method which I don't like. But I can't find a way to get the actual old position of the element. Due to the remove of the city object, the move operation gets more complicated. I don't understand why there is a move in the first place. All it has to do is delete one city and that's it. But I'm going to open another question for this as this is off-topic.