CRASH attempt to delete and reload the same index path

Question

CollectionViewController.m line 439 __50-[CollectionViewController photoLibraryDidChange:]_block_invoke

Fatal Exception: NSInternalInconsistencyException attempt to delete and reload the same index path ( {length = 2, path = 0 - 26007})

- (void)photoLibraryDidChange:(PHChange *)changeInstance
{
    // Call might come on any background queue. Re-dispatch to the main queue to handle it.
    dispatch_async(dispatch_get_main_queue(), ^{

        // check if there are changes to the assets (insertions, deletions, updates)
        PHFetchResultChangeDetails *collectionChanges = [changeInstance changeDetailsForFetchResult:self.assetsFetchResults];
        if (collectionChanges) {

            // get the new fetch result
            self.assetsFetchResults = [collectionChanges fetchResultAfterChanges];

            UICollectionView *collectionView = self.collectionView;

            if (![collectionChanges hasIncrementalChanges] || [collectionChanges hasMoves]) {
                // we need to reload all if the incremental diffs are not available
                [collectionView reloadData];

            } else {
                // if we have incremental diffs, tell the collection view to animate insertions and deletions
                [collectionView performBatchUpdates:^{
                    NSIndexSet *removedIndexes = [collectionChanges removedIndexes];
                    if ([removedIndexes count]) {
                        [collectionView deleteItemsAtIndexPaths:[removedIndexes aapl_indexPathsFromIndexesWithSection:0]];
                    }
                    NSIndexSet *insertedIndexes = [collectionChanges insertedIndexes];
                    if ([insertedIndexes count]) {
                        [collectionView insertItemsAtIndexPaths:[insertedIndexes aapl_indexPathsFromIndexesWithSection:0]];
                    }
                    NSIndexSet *changedIndexes = [collectionChanges changedIndexes];
                    if ([changedIndexes count]) {
                        [collectionView reloadItemsAtIndexPaths:[changedIndexes aapl_indexPathsFromIndexesWithSection:0]];
                    }
                } completion:NULL];
            }

            [self resetCachedAssets];
        }
    });
}

source: https://developer.apple.com/devcenter/download.action?path=/wwdc_2014/wwdc_2014_sample_code/exampleappusingphotosframework.zip

I can't replicate the issue. What could be the problem? Thanks a lot!

Answer 1

I was able to reproduce this today. To do this you need to:

1. Open your app that is listening for changes
2. Open the photos app, save a set of photos to your photo library from an iCloud shared album
3. Go to the photos app, delete some of those photos
4. Go again to the iCloud shared album and save again the some of the photos you deleted. You'll see this condition happen.

I found an updated code that seems to work better to handle the updating behavior here: https://developer.apple.com/library/ios/documentation/Photos/Reference/PHPhotoLibraryChangeObserver_Protocol/

But it still doesn't handle this situation nor when the indexes to be deleted are bigger (i.e. Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempt to delete item 9 from section 0 which only contains 9 items before the update'). I created this updated version of this code that deals with this better and hasn't crashed for me anymore so far.

func photoLibraryDidChange(changeInfo: PHChange!) {

    // Photos may call this method on a background queue;
    // switch to the main queue to update the UI.
    dispatch_async(dispatch_get_main_queue()) {


        // Check for changes to the list of assets (insertions, deletions, moves, or updates).
        if let collectionChanges = changeInfo.changeDetailsForFetchResult(self.assetsFetchResult) {

            // Get the new fetch result for future change tracking.
            self.assetsFetchResult = collectionChanges.fetchResultAfterChanges

            if collectionChanges.hasIncrementalChanges {

                // Get the changes as lists of index paths for updating the UI.
                var removedPaths: [NSIndexPath]?
                var insertedPaths: [NSIndexPath]?
                var changedPaths: [NSIndexPath]?
                if let removed = collectionChanges.removedIndexes {
                    removedPaths = self.indexPathsFromIndexSetWithSection(removed,section: 0)
                }
                if let inserted = collectionChanges.insertedIndexes {
                    insertedPaths = self.indexPathsFromIndexSetWithSection(inserted,section: 0)
                }
                if let changed = collectionChanges.changedIndexes {
                    changedPaths = self.indexPathsFromIndexSetWithSection(changed,section: 0)
                }
                var shouldReload = false
                if changedPaths != nil && removedPaths != nil{
                    for changedPath in changedPaths!{
                        if contains(removedPaths!,changedPath){
                            shouldReload = true
                            break
                        }
                    }

                }

                if removedPaths?.last?.item >= self.assetsFetchResult.count{
                    shouldReload = true
                }

                if shouldReload{
                    self.collectionView.reloadData()
                }else{
                    // Tell the collection view to animate insertions/deletions/moves
                    // and to refresh any cells that have changed content.
                    self.collectionView.performBatchUpdates(
                        {
                            if let theRemovedPaths = removedPaths {
                                self.collectionView.deleteItemsAtIndexPaths(theRemovedPaths)
                            }
                            if let theInsertedPaths = insertedPaths {
                                self.collectionView.insertItemsAtIndexPaths(theInsertedPaths)
                            }
                            if let theChangedPaths = changedPaths{
                                self.collectionView.reloadItemsAtIndexPaths(theChangedPaths)
                            }
                            if (collectionChanges.hasMoves) {
                                collectionChanges.enumerateMovesWithBlock() { fromIndex, toIndex in
                                    let fromIndexPath = NSIndexPath(forItem: fromIndex, inSection: 0)
                                    let toIndexPath = NSIndexPath(forItem: toIndex, inSection: 0)
                                    self.collectionView.moveItemAtIndexPath(fromIndexPath, toIndexPath: toIndexPath)
                                }
                            }
                        }, completion: nil)

                }

            } else {
                // Detailed change information is not available;
                // repopulate the UI from the current fetch result.
                self.collectionView.reloadData()
            }
        }
    }
}

func indexPathsFromIndexSetWithSection(indexSet:NSIndexSet?,section:Int) -> [NSIndexPath]?{
    if indexSet == nil{
        return nil
    }
    var indexPaths:[NSIndexPath] = []

    indexSet?.enumerateIndexesUsingBlock { (index, Bool) -> Void in
        indexPaths.append(NSIndexPath(forItem: index, inSection: section))
    }
    return indexPaths

}

Swift 3 / iOS 10 version:

func photoLibraryDidChange(_ changeInstance: PHChange) {
    guard let collectionView = self.collectionView else {
        return
    }

    // Photos may call this method on a background queue;
    // switch to the main queue to update the UI.
    DispatchQueue.main.async {
        guard let fetchResults = self.fetchResults else {
            collectionView.reloadData()
            return
        }

        // Check for changes to the list of assets (insertions, deletions, moves, or updates).
        if let collectionChanges = changeInstance.changeDetails(for: fetchResults) {
            // Get the new fetch result for future change tracking.
            self.fetchResults = collectionChanges.fetchResultAfterChanges

            if collectionChanges.hasIncrementalChanges {
                // Get the changes as lists of index paths for updating the UI.
                var removedPaths: [IndexPath]?
                var insertedPaths: [IndexPath]?
                var changedPaths: [IndexPath]?
                if let removed = collectionChanges.removedIndexes {
                    removedPaths = self.indexPaths(from: removed, section: 0)
                }
                if let inserted = collectionChanges.insertedIndexes {
                    insertedPaths = self.indexPaths(from:inserted, section: 0)
                }
                if let changed = collectionChanges.changedIndexes {
                    changedPaths = self.indexPaths(from: changed, section: 0)
                }
                var shouldReload = false
                if let removedPaths = removedPaths, let changedPaths = changedPaths {
                    for changedPath in changedPaths {
                        if removedPaths.contains(changedPath) {
                            shouldReload = true
                            break
                        }
                    }
                }

                if let item = removedPaths?.last?.item {
                    if item >= fetchResults.count {
                        shouldReload = true
                    }
                }

                if shouldReload {
                    collectionView.reloadData()
                } else {
                    // Tell the collection view to animate insertions/deletions/moves
                    // and to refresh any cells that have changed content.
                    collectionView.performBatchUpdates({
                        if let theRemovedPaths = removedPaths {
                            collectionView.deleteItems(at: theRemovedPaths)
                        }
                        if let theInsertedPaths = insertedPaths {
                            collectionView.insertItems(at: theInsertedPaths)
                        }
                        if let theChangedPaths = changedPaths {
                            collectionView.reloadItems(at: theChangedPaths)
                        }

                        collectionChanges.enumerateMoves { fromIndex, toIndex in
                            collectionView.moveItem(at: IndexPath(item: fromIndex, section: 0),
                                                    to: IndexPath(item: toIndex, section: 0))
                        }
                    })
                }
            } else {
                // Detailed change information is not available;
                // repopulate the UI from the current fetch result.
                collectionView.reloadData()
            }
        }
    }
}

func indexPaths(from indexSet: IndexSet?, section: Int) -> [IndexPath]? {
    guard let set = indexSet else {
        return nil
    }

    return set.map { (index) -> IndexPath in
        return IndexPath(item: index, section: section)
    }
}

Answer 2

I just moved the reloadItemsAtIndexPaths after the batch updates are completed to fix the crash of deleting and reloading at the same time.

From docs of changedIndexes of PHFetchResultChangeDetails:

These indexes are relative to the original fetch result (the fetchResultBeforeChanges property) after you’ve applied the changes described by the removedIndexes and insertedIndexes properties; when updating your app’s interface, apply changes after removals and insertions and before moves.

PHFetchResultChangeDetails *collectionChanges = [changeInstance changeDetailsForFetchResult:self.assetsFetchResults];
[collectionView performBatchUpdates:^{ 
        NSIndexSet *removedIndexes = [collectionChanges removedIndexes];
        if ([removedIndexes count]) {
            [collectionView deleteItemsAtIndexPaths:[self indexPathsFromIndexes:removedIndexes withSection:0]];
        }
        NSIndexSet *insertedIndexes = [collectionChanges insertedIndexes];
        if ([insertedIndexes count]) {
            [collectionView insertItemsAtIndexPaths:[self indexPathsFromIndexes:insertedIndexes withSection:0]];
        }
    } completion:^(BOOL finished) {
        if (finished) {
            // Puting this after removes and inserts indexes fixes a crash of deleting and reloading at the same time.
            // From docs: When updating your app’s interface, apply changes after removals and insertions and before moves.
            NSIndexSet *changedIndexes = [collectionChanges changedIndexes];
            if ([changedIndexes count]) {
                [collectionView reloadItemsAtIndexPaths:[self indexPathsFromIndexes:changedIndexes withSection:0]];
            }
       }
    }

Answer 3

I implemented the code in batkryu's answer in Objective-C.

- (void)photoLibraryDidChange:(PHChange *)changeInstance {

    dispatch_async(dispatch_get_main_queue(), ^{

        PHFetchResultChangeDetails *collectionChanges = [changeInstance changeDetailsForFetchResult:self.assetsFetchResults];
        if (collectionChanges) {

            self.assetsFetchResults = [collectionChanges fetchResultAfterChanges];

            UICollectionView *collectionView = self.collectionView;
            NSArray *removedPaths;
            NSArray *insertedPaths;
            NSArray *changedPaths;

            if ([collectionChanges hasIncrementalChanges]) {
                NSIndexSet *removedIndexes = [collectionChanges removedIndexes];
                removedPaths = [self indexPathsFromIndexSet:removedIndexes withSection:0];

                NSIndexSet *insertedIndexes = [collectionChanges insertedIndexes];
                insertedPaths = [self indexPathsFromIndexSet:insertedIndexes withSection:0];

                NSIndexSet *changedIndexes = [collectionChanges changedIndexes];
                changedPaths = [self indexPathsFromIndexSet:changedIndexes withSection:0];

                BOOL shouldReload = NO;

                if (changedPaths != nil && removedPaths != nil) {
                    for (NSIndexPath *changedPath in changedPaths) {
                        if ([removedPaths containsObject:changedPath]) {
                            shouldReload = YES;
                            break;
                        }
                    }
                }

                if (removedPaths.lastObject && ((NSIndexPath *)removedPaths.lastObject).item >= self.assetsFetchResults.count) {
                    shouldReload = YES;
                }

                if (shouldReload) {
                    [collectionView reloadData];

                } else {
                    [collectionView performBatchUpdates:^{
                        if (removedPaths) {
                            [collectionView deleteItemsAtIndexPaths:removedPaths];
                        }

                        if (insertedPaths) {
                            [collectionView insertItemsAtIndexPaths:insertedPaths];
                        }

                        if (changedPaths) {
                            [collectionView reloadItemsAtIndexPaths:changedPaths];
                        }

                        if ([collectionChanges hasMoves]) {
                            [collectionChanges enumerateMovesWithBlock:^(NSUInteger fromIndex, NSUInteger toIndex) {
                                NSIndexPath *fromIndexPath = [NSIndexPath indexPathForItem:fromIndex inSection:0];
                                NSIndexPath *toIndexPath = [NSIndexPath indexPathForItem:toIndex inSection:0];
                                [collectionView moveItemAtIndexPath:fromIndexPath toIndexPath:toIndexPath];
                            }];
                        }

                    } completion:NULL];
                }

                [self resetCachedAssets];
            } else {
                [collectionView reloadData];
            }
        }
    });
}

- (NSArray *)indexPathsFromIndexSet:(NSIndexSet *)indexSet withSection:(int)section {
    if (indexSet == nil) {
        return nil;
    }
    NSMutableArray *indexPaths = [[NSMutableArray alloc] init];

    [indexSet enumerateIndexesUsingBlock:^(NSUInteger idx, BOOL *stop) {
        [indexPaths addObject:[NSIndexPath indexPathForItem:idx inSection:section]];
    }];

    return indexPaths;
}

Answer 4

Finally read documentation (not from developer.apple.com, but in Xcode) and the right way to apply incremental changes is:

collectionView.performBatchUpdates({
    // removedIndexes
    // insertedIndexes
}, completion: {
    // changedIndexes (using collectionView.cellForItem(at: index))
    collectionView.performBatchUpdates({
        // enumerateMoves
    })
})

Before implementing this approach we had random crashes when items were moved.

Why this is correct?

"removedIndexes" and "insertedIndexes" go first because this indexes are relative to the original fetch result

"removedIndexes" - "indexes are relative to the original fetch result (the fetchResultBeforeChanges property); when updating your app’s interface, apply removals before insertions, changes, and moves." "insertedIndexes" - "indexes are relative to the original fetch result (the fetchResultBeforeChanges property) after you’ve applied the changes described by the removedIndexes property; when updating your app’s interface, apply insertions after removals and before changes and moves."

"changedIndexes" can't be processed with "delete"/"insert" because they describe items after insertions/deletions

"changedIndexes" - "These indexes are relative to the original fetch result (the fetchResultBeforeChanges property) after you’ve applied the changes described by the removedIndexes and insertedIndexes properties; when updating your app’s interface, apply changes after removals and insertions and before moves.

Warning Don't map changedIndexes directly to UICollectionView item indices in batch updates. Use these indices to reconfigure the corresponding cells after performBatchUpdates(_:completion:). UICollectionView and UITableView expect the changedIndexes to be in the before state, while PhotoKit provides them in the after state, resulting in a crash if your app performs insertions and deletions at the same time as the changes."

"enumerateMoves" is the last thing we should do.

"enumerateMoves" - "The toIndex parameter in the handler block is relative to the state of the fetch result after you’ve applied the changes described by the removedIndexes, insertedIndexes and changedIndexes properties. Therefore, if you use this method to update a collection view or similar user interface displaying the contents of the fetch result, update your UI to reflect insertions, removals, and changes before you process moves."

Answer 5

This is an improvement to @batkru's answer, which eliminates the need for the variable shouldReload:

func photoLibraryDidChange(changeInstance: PHChange) {
    dispatch_async(dispatch_get_main_queue(), {
        let changeDetails = changeInstance.changeDetailsForFetchResult(self.assetsFetchResult)

        if let details = changeDetails {
            self.assetsFetchResult = details.fetchResultAfterChanges

            if details.hasIncrementalChanges {
                var removedIndexes: [NSIndexPath]?
                var insertedIndexes: [NSIndexPath]?
                var changedIndexes: [NSIndexPath]?

                if let removed = details.removedIndexes {
                    removedIndexes = createIndexPathsFromIndices(removed)
                }
                if let inserted = details.insertedIndexes {
                    insertedIndexes = createIndexPathsFromIndices(inserted)
                }
                if let changed = details.changedIndexes {
                    changedIndexes = createIndexPathsFromIndices(changed)
                }

                if removedIndexes != nil && changedIndexes != nil {
                    for removedIndex in removedIndexes! {
                        let indexOfAppearanceOfRemovedIndexInChangedIndexes = find(changedIndexes!, removedIndex)
                        if let index = indexOfAppearanceOfRemovedIndexInChangedIndexes {
                            changedIndexes!.removeAtIndex(index)
                        }
                    }
                }

                self.collectionView?.performBatchUpdates({
                    if let removed = removedIndexes {
                        self.collectionView?.deleteItemsAtIndexPaths(removed)
                    }
                    if let inserted = insertedIndexes {
                        self.collectionView?.insertItemsAtIndexPaths(inserted)
                    }
                    if let changed = changedIndexes {
                        self.collectionView?.reloadItemsAtIndexPaths(changed)
                    }
                    if details.hasMoves {
                        changeDetails!.enumerateMovesWithBlock({ fromIndex, toIndex in
                            self.collectionView?.moveItemAtIndexPath(NSIndexPath(forItem: fromIndex, inSection: 0), toIndexPath: NSIndexPath(forItem: toIndex, inSection: 0))
                        })
                    }
                }, completion: nil)
            } else {
                self.collectionView?.reloadData()
            }
        }
    })
}

Answer 6

So I did well with @FernandoEscher's translation of @batkryu's solution, except in the situation where an iCloud Photo Library with tons of changes was recently re-conneted. In this situation the collection becomes totally un-responsive and can crash. The core problem is that photoLibraryDidChange will get called again before the performBatchUpdates completion fires. The call to performBatchUpdates before a performBatchUpdates finishes seems to kill performance. I suspect that the crash happens because assetsFetchResults gets modified while the animation is running for its previous value.

Sooooo, here's what I did:

elsewhere in the init....

self.phPhotoLibChageMutex = dispatch_semaphore_create(1);

_

- (void)photoLibraryDidChange:(PHChange *)changeInstance {
    dispatch_semaphore_wait(self.phPhotoLibChageMutex, DISPATCH_TIME_FOREVER);

    dispatch_async(dispatch_get_main_queue(), ^{

        PHFetchResultChangeDetails *collectionChanges = [changeInstance changeDetailsForFetchResult:self.assetsFetchResults];
        if (collectionChanges) {

            self.assetsFetchResults = [collectionChanges fetchResultAfterChanges];

            UICollectionView *collectionView = self.collectionView;
            NSArray *removedPaths;
            NSArray *insertedPaths;
            NSArray *changedPaths;

            if ([collectionChanges hasIncrementalChanges]) {
                NSIndexSet *removedIndexes = [collectionChanges removedIndexes];
                removedPaths = [self indexPathsFromIndexSet:removedIndexes withSection:0];

                NSIndexSet *insertedIndexes = [collectionChanges insertedIndexes];
                insertedPaths = [self indexPathsFromIndexSet:insertedIndexes withSection:0];

                NSIndexSet *changedIndexes = [collectionChanges changedIndexes];
                changedPaths = [self indexPathsFromIndexSet:changedIndexes withSection:0];

                BOOL shouldReload = NO;

                if (changedPaths != nil && removedPaths != nil) {
                    for (NSIndexPath *changedPath in changedPaths) {
                        if ([removedPaths containsObject:changedPath]) {
                            shouldReload = YES;
                            break;
                        }
                    }
                }

                if (removedPaths.lastObject && ((NSIndexPath *)removedPaths.lastObject).item >= self.assetsFetchResults.count) {
                    shouldReload = YES;
                }

                if (shouldReload) {
                    [collectionView reloadData];
                    [self fixupSelection];
                    dispatch_semaphore_signal(self.phPhotoLibChageMutex);
                } else {
                    [collectionView performBatchUpdates:^{
                        if (removedPaths) {
                            [collectionView deleteItemsAtIndexPaths:removedPaths];
                        }

                        if (insertedPaths) {
                            [collectionView insertItemsAtIndexPaths:insertedPaths];
                        }

                        if (changedPaths) {
                            [collectionView reloadItemsAtIndexPaths:changedPaths];
                        }

                        if ([collectionChanges hasMoves]) {
                            [collectionChanges enumerateMovesWithBlock:^(NSUInteger fromIndex, NSUInteger toIndex) {
                                NSIndexPath *fromIndexPath = [NSIndexPath indexPathForItem:fromIndex inSection:0];
                                NSIndexPath *toIndexPath = [NSIndexPath indexPathForItem:toIndex inSection:0];
                                [collectionView moveItemAtIndexPath:fromIndexPath toIndexPath:toIndexPath];
                            }];
                        }

                    } completion:^(BOOL finished) {
                        [self fixupSelection];
                        dispatch_semaphore_signal(self.phPhotoLibChageMutex);
                    }];
                }

                [self resetCachedAssets];
            } else {
                [collectionView reloadData];
                [self fixupSelection];
                dispatch_semaphore_signal(self.phPhotoLibChageMutex);
            }
        }else{
            dispatch_semaphore_signal(self.phPhotoLibChageMutex);
        }
    });
}

- (NSArray *)indexPathsFromIndexSet:(NSIndexSet *)indexSet withSection:(int)section {
    if (indexSet == nil) {
        return nil;
    }
    NSMutableArray *indexPaths = [[NSMutableArray alloc] init];

    [indexSet enumerateIndexesUsingBlock:^(NSUInteger idx, BOOL *stop) {
        [indexPaths addObject:[NSIndexPath indexPathForItem:idx inSection:section]];
    }];

    return indexPaths;
}