Crash when update occurs as side effect of view configuration

[JosephDuffy]

A crash can happen when an update occurs as a side effect of a view being configured, e.g. the following will trigger a crash:

• Batch update with insert 0, 0
• View for item 0, 0 deletes item 0, 0 during configuration

A somewhat more reasonable example would be something in a header (e.g. a series of tabs) triggering the update as a setup of initial state.

This is because the updates closure of performBatchUpdates is still being called so the delete is processed in the context prior to the insert. This is generally a misuse of UICollectionView and is hard to create a correct fix.

Consumers should be discouraged from performing updates during configuration, but it would be nice if this could be detected and the crash prevented.

Original message

I'm trying to debug batch updates and seeing some strange behaviour, maybe you can provide some insight @shaps80?

The updates are applied in https://github.com/opennetltd/Composed/blob/batch-collection-view-updates-closure/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift#L254, which can be simplified as:

print("Calling `performBatchUpdates`")
collectionView.performBatchUpdates({
    print("Starting batch updates")
    // ... apply updates
    print("Batch updates have been applied")
}, completion: { [weak self] isFinished in
    print("Batch updates completed. isFinished: \(isFinished)")
})
print("`performBatchUpdates` has been called")

I have then seen logs like:

Calling `performBatchUpdates`
Starting batch updates
Batch updates have been applied

Calling `performBatchUpdates`
Starting batch updates
Batch updates have been applied

`performBatchUpdates` has been called
`performBatchUpdates` has been called

Calling `performBatchUpdates`
Starting batch updates
Batch updates have been applied

**Crash**

Note how Starting batch updates is printed twice before the `performBatchUpdates` has been called. How can this be possible?

This made me look at this a little more, and only raised more questions. The updates closure has to be sync because it's not escaping (although I don't see this in the actual header and it is optional, but somehow Swift thinks it's not escaping) and Batch updates have been applied is always printed before another update.

It looks like collection view is blocking the return from performBatchUpdates and multiple calls are possible, but this is all being done on the main thread. This makes me think my method of testing is wrong but I can't see what it could be.

Note that the crash occurs when updates are applied as:

• Insert section 0
• Insert element 0,0
• Reload element 0,0

Since the completion hasn't been called yet I think it hasn't committed the insert yet and it crashes with a message stating that it can't delete element 0,0 because it doesn't exist yet.

One solution is to wait for completion to be called before applying more updates but this would then make all updates within Composed potentially async and introduce delay (due to waiting for layout) that isn't required the majority of the time.

[shaps80]

This doesn't really make sense. The whole point of calling performBatchUpdates for every single update is to ensure it enqueues those correctly. So it shouldn't be possible to NOT have index-0. That being said I do remember reading somewhere that the call to performBatchUpdates is sync while the internal execution is async. So it'll return immediately but potentially execute later. I believe this is to ensure you can call this quite rapidly without issue.

The most common cause of issues here is usually the data being out of sick. Are you updating the model inside the performBatchUpdates as well, you should be?

[JosephDuffy]

The most common cause of issues here is usually the data being out of sick. Are you updating the model inside the performBatchUpdates as well, you should be?

Moving to the closure-based design is aimed to ensure that the models are updated inside performBatchUpdates (Composed is currently not guaranteeing this).

---

One way this crash seems to happen is when the configuration of a header (possibly cells, I've not tested this yet) triggers an update. This is probably impossible to support without capturing the updates closure and I think should be classed as a misuse of the API (note that I wrote part of this 😬).

The workaround I've found (after a few separate debugging sessions) is to invalidate all if this scenario is detected:

• performBatchUpdates has been called on the collection view
• All updates have been applied to the collection view

After the updates have been applied the collection will layout, dequeuing views as necessary. If any of these views then trigger an update it will enter CollectionCoordinator.mapping(_:willPerformBatchUpdates:) again, so the logs make sense now.

I've added a long print statement when this happens so consumers can debug this a little easier. Invalidating everything fixes the crash so I think this is a good compromise?

At first I thought this might work because collection view apparently supports nested batch updates:

But I would guess their logic is along the lines of:

• Call updates
• Dequeue views
• Commit changes
• async wait for animations
• Call completion

Because the crash is essentially:

• Call updates
  • updates inserts 0,0
• Dequeue views
  • Dequeuing deletes 0,0
• Crash because 0,0 doesn't exist