Bug when a section reload is queued while batch updating

[rnystrom]

I discovered this working on my sample weather app. Here's the conditions that I noticed to reproduce this:

• Add a Dispatch.async { ... } inside didUpdate(to object:)
• In the block, call collectionContext.reload(section: self)
  • Note do not use batch updating

Result: reload will never be called.

This is because -[UICollectionView performBatchUpdates:completion:], is asynchronous in that it executes over several runloop turns (aggregation and animations).

We use a flag to monitor when batch updating, setting it to YES before we enter the update block, then setting it to NO in the completion block of performBatchUpdates:completion:.

If a reload is added while this flag is set to YES, we assume it is part of unloading our blocks of batched operations.

The problem is anyone can queue a reload from anywhere. When the current batch updates finish, we queue another update to clear anything we might have collected in the meanwhile. However before we update we check if hasChanges is true.

Unfortunately the bug lies in that we don't check reloads in hasChanges.

Some options for fixing:

• Check self.reloadSections.count > 0 in hasChanges
  • Quickest fix
• Replace batchUpdateOrReloadInProgress with a "mode" enum to model states of performBatchUpdates:completion::
  • Just before calling
  • Enter/exit update block
  • Enter/exit completion block
  • Finished (technically on completion block exit)

The latter is a more thorough solution and probably better in the long run given some of the plans I have for how batch operations are handled.

[jessesquires]

@rnystrom should we get this in 2.0 or release a 2.0.1 ?

[rnystrom]

@jessesquires let's do 2.0.1, pretty difficult/odd setup to get this to happen, and we've never seen it in IG, so I think we can take some time to do this one right.

[vldalx]

@rnystrom, Hi.
I've faced a crash. It looks like a reason for the crash the same as described above.

I'm trying to implement a chat using IGListKit. Messages should be grouped by date. My app fetches messages' batches from a server side while a user scrolls the chat.

So, I use SectionController as a messages' group.
If all new messages should be added into an existing group I add them to a datasource and invoke collectionContext!.insert method using IndexSet
If a new section or new sections should be added I add them to a datasource and invoke adapter.performUpdates method

Sometimes, if a user scrolls very fast my app will crash.
There is a methods' calls flow:

1. adapter.performUpdates - the first messages' batch is loaded
2. a few collectionContext!.insert - a few batches are appended to the end of the first section
3. adapter.performUpdates - 3 new sections are added
4. collectionContext!.insert - a new messages' batch is appended to the end of the last section
5. collectionContext!.insert - a new messages' batch is appended to the end of the last section -> CRASH

My investigation shows, actually 4 step was not executed. New cells were not inserted in UICollectionView. And on the 5 step I get a crash, because of I try insert a new batch, but UICollectionView doesn't about previously "added" batch.

Otherwise, if a user scrolls slow, everything works. I think the reason is a time interval between steps 3 and 4 is longer in that case.

If I invokecollectionContext!.insert method within section batch updating everything works nice.

[rnystrom]

@vldalx that sounds like it might be a separate issue. I'd love if you could:

• File a new issue so we can track this
• Give some examples of your setup (e.g. your IGListAdapterDataSource, section controllers, and when/how the section controllers insert new messages)
• (optional but extremely helpful) An example project demoing the bug

Sounds like you might have found something really interesting!