ASDataController locks datasource multiple times when processing multiple changes

[smeis]

I've run into a problem using ASCollectionView which is similar to what @ryanfitz describes here: #344 (comment) and is probably a bug.

When you attempt to combine one of the update methods with another one the app will crash. ASDataController will call dataControllerLockDataSource from performDataFetchingWithBlock: on its data source for each group of changes (so once for deleteItemsAtIndexPaths:, once for insertItemsAtIndexPaths: and once for reloadItemsAtIndexPaths:). After the first call the datasource is already locked which will result in a crash on the second call because of the Assert in dataControllerLockDataSource in ASCollectionView.

I'm assuming this is not intended behavior. To solve this I've added a lock counter to ASCollectionView which gets incremented in dataControllerLockDataSource and only locks asyncDataSource on the first call. The counter gets decremented in dataControllerUnlockDataSource and unlocks asyncDataSource when the counter reaches zero.

I can create a PR if you guys think this is a good way to fix this.

EDIT: Just realized that in my solution it would probably make a lot more sense to add the counter to ASDataController.

[secretiverhyme]

Yes, this sounds like a bug — thanks for the report! We're planning on revising if not removing the data source locking behaviour before releasing the AsyncDisplayKit 1.2 beta (#338).

[smeis]

Ah I see, I've been meaning to join the discussion in #338 but kept forgetting it. I'll write something up right now because apart from this little bug I'm very happy with the asynchronous fetching!

[appleguy]

@smeis thanks for reporting this; as of yesterday this area of the code has been radically refactored, and I believe this problem is either fixed or structurally eliminated. If you are able to break the code in any way, please file a new issue — I would love to hear about it and make it a top priority to resolve.

Sorry for the long delay on the issues around this code. It was a complicated area to rewrite, and a lot of factors came together to make it difficult to give the time dedication needed, but now that the technical debt is eliminated I plan on keeping it pretty spotless.

[smeis]

Ok thanks for fixing this @appleguy ! I saw the PR for your rewrite and was planning to test it but hadn't gotten around to it. Will test it this week and will of course report any problems!

[hanweixing]

I met this problem too, and i use latest version Texture.
I submit an issue: TextureGroup/Texture#1266
Could anyone help to figure it out? Appreciate, thank you!