add assert to avoid UICollectionView data inconsistencies

Summary:
A common cause of `NSInternalInconsistencyException` is when a section controller tries to access a cell or modify the `UICollectionView` in `-didUpdateToObject`. If the collection view doesn't have data yet, this will kick off the regular calls (ex: `-numberOfSectionsInCollectionView`, `-numberOfItemsInSection`), which won't have the right information yet because the `IGListAdpater` is in the middle of updating the object list. Eventually, this can lead to a crash, which are hard to debug because the callstack doesn't include the call that caused the premature update.

Lets add an assert to catch when this happens.

Reviewed By: candance

Differential Revision: D20527372

fbshipit-source-id: 4ad0b2d89e5b126d6ae888a5c3b1d980945b7a4c
This commit is contained in:
Maxime Ollivier 2020-03-19 10:10:01 -07:00 committed by Facebook GitHub Bot
parent 68e58bf60e
commit 0f3fd694d3
3 changed files with 21 additions and 1 deletions

View file

@ -598,6 +598,9 @@
- (void)_updateObjects:(NSArray *)objects dataSource:(id<IGListAdapterDataSource>)dataSource {
IGParameterAssert(dataSource != nil);
// Should be the first thing called in this function.
_isInObjectUpdateTransaction = YES;
#if DEBUG
for (id object in objects) {
IGAssert([object isEqualToDiffableObject:object], @"Object instance %@ not equal to itself. This will break infra map tables.", object);
@ -661,6 +664,9 @@
}
[self _updateBackgroundViewShouldHide:![self _itemCountIsZero]];
// Should be the last thing called in this function.
_isInObjectUpdateTransaction = NO;
}
- (void)_updateBackgroundViewShouldHide:(BOOL)shouldHide {

View file

@ -12,15 +12,20 @@
#import <IGListKit/IGListSectionController.h>
#import <IGListKit/IGListSectionControllerInternal.h>
#import "IGListAdapterInternal.h"
@implementation IGListAdapter (UICollectionView)
#pragma mark - UICollectionViewDataSource
- (NSInteger)numberOfSectionsInCollectionView:(UICollectionView *)collectionView {
_assertNotInMiddleOfObjectUpdate(self.isInObjectUpdateTransaction);
return self.sectionMap.objects.count;
}
- (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSection:(NSInteger)section {
_assertNotInMiddleOfObjectUpdate(self.isInObjectUpdateTransaction);
IGListSectionController * sectionController = [self sectionControllerForSection:section];
IGAssert(sectionController != nil, @"Nil section controller for section %li for item %@. Check your -diffIdentifier and -isEqual: implementations.",
(long)section, [self.sectionMap objectForSection:section]);
@ -304,4 +309,10 @@
return attributes;
}
#pragma mark - Assert helpers
static void _assertNotInMiddleOfObjectUpdate(BOOL isInObjectUpdateTransaction) {
IGAssert(isInObjectUpdateTransaction == NO, @"The UICollectionView is attempting to update its data while the IGListAdapter is in a middle of updating the object list. This will cause inconsistencies and potentially crashes (which are hard to debug). The most common cause is a section controller tries to access/modify the UICollectionView in -didUpdateToObject.");
}
@end

View file

@ -43,9 +43,12 @@ IGListBatchContext
@property (nonatomic, strong, nullable) UIView *emptyBackgroundView;
// we need to special case interactive section moves that are moved to the last position
// We need to special case interactive section moves that are moved to the last position
@property (nonatomic) BOOL isLastInteractiveMoveToLastSectionIndex;
// We're in the middle of updating the objects.
@property (nonatomic) BOOL isInObjectUpdateTransaction;
/**
When making object updates inside a batch update block, delete operations must use the section /before/ any moves take
place. This includes when other objects are deleted or inserted ahead of the section controller making the mutations.