Crash when invalidating then inserting/deleting

[JosephDuffy]

Describe the bug

A crash can occur when using invalidateAll followed by an update (I've only triggered this with an insert but others could work, I suspect delete too).

To Reproduce

Call invalidateAll, followed by an insert/delete. I think the invalidateAll call needs to trigger a layout.

Expected behavior

It should not crash.

Environment

• OS Version: iOS 13.7
• Library Version: master (8ee5b11, 1.0.3 + some fixes)
• Device: iPod Touch

Additional context

I think the issue comes down to how the API for the delegate and UICollectionView itself differ. My understanding is that calling reloadData (which is done on an invalidate) does not guarantee that the changes have been fully applied after reloadData occurs, e.g. a layout pass might be required.

When looking at the docs for performBatchUpdates it states:

If the collection view's layout is not up to date before you call this method, a reload may occur. To avoid problems, you should update your data model inside the updates block or ensure the layout is updated before you call performBatchUpdates(_:completion:).

This would be ok if we were updating the data model inside the updates block, but updates are surrounded by willBeginUpdating and didEndUpdating. didEndUpdating triggers the call to performBatchUpdates but by this point the models aren't in-sync with what the collection view thinks is true.

So I think when we call reloadData and then performBatchUpdates before the layout has occurred the collection view will try to update straight away. This starts with numberOfSections which will return the new value, but when the collection view then requires for a cell it will eventually hit elementsProvider(for:) which will crash because the cachedProviders has not been updated yet because the prepareSections call occurs inside the performBatchUpdates.

I tried calling layoutIfNeeded before performBatchUpdates. This essentially triggers the same crash since I think that's what performBatchUpdates is doing internally

I also tried calling collectionView.layoutIfNeeded() straight after collectionView.reloadData() which fixes the crash but seems to cause a visual bug so I'm not sure if this can be classed as a fix or not.

Ultimately the fix for me was to remove invalidateAll and do diffing, but since most of the built-in types use invalidateAll this isn't really a fix either.

Ultimately I think the fix is to update the delegate in such a way that the model updates can occur inside the performBatchUpdates call, but this would need some more thought an a major version bump.

[shaps80]

Major version bump why? Otherwise I can see the issue.

[JosephDuffy]

Major version bump why? Otherwise I can see the issue.

I don't see a way of fixing this with the existing API so it would have to replace the existing delegate (major version bump) or a new delegate be introduced and this issue be documented.