Skip to content

Commit

Permalink
add assert for iOS 18 cell dequeue exceptions
Browse files Browse the repository at this point in the history
Summary:
iOS 18 has new exceptions, which can be a bit tough to debug. The common ones we see:
1. Cell dequeue outside of -cellForIndex
2. Dequeue being called more than once
3. Returning a cell that was not dequeued

Lets add an assert for each. #1 and #2 can actually assert on the problematic call, which will give a nice stack strace. #3 will still have a generic stack trace, but at least we'll see the section-controller.

Differential Revision: D62809660

fbshipit-source-id: 454c44598d4b21e09e6f66eb63c502b1b966a993
  • Loading branch information
Maxime Ollivier authored and facebook-github-bot committed Sep 17, 2024
1 parent 5ee2207 commit ab94446
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 3 deletions.
8 changes: 8 additions & 0 deletions Source/IGListDiffKit/IGListAssert.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,11 @@
#ifndef IGAssertMainThread
#define IGAssertMainThread() IGAssert( ([NSThread isMainThread] == YES), @"Must be on the main thread")
#endif // IGAssertMainThread

#ifndef IG_ASSERTIONS_ENABLED
#if !defined(NS_BLOCK_ASSERTIONS)
#define IG_ASSERTIONS_ENABLED 1
#else
#define IG_ASSERTIONS_ENABLED 0
#endif
#endif
18 changes: 15 additions & 3 deletions Source/IGListKit/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ - (__kindof UICollectionViewCell *)dequeueReusableCellOfClass:(Class)cellClass
[self.registeredCellIdentifiers addObject:identifier];
[collectionView registerClass:cellClass forCellWithReuseIdentifier:identifier];
}
return [collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath];
return [self _dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath forSectionController:sectionController];
}

- (__kindof UICollectionViewCell *)dequeueReusableCellOfClass:(Class)cellClass
Expand All @@ -1172,7 +1172,7 @@ - (__kindof UICollectionViewCell *)dequeueReusableCellFromStoryboardWithIdentifi
UICollectionView *collectionView = self.collectionView;
IGAssert(collectionView != nil, @"Reloading adapter without a collection view.");
NSIndexPath *indexPath = [self indexPathForSectionController:sectionController index:index usePreviousIfInUpdateBlock:NO];
return [collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath];
return [self _dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath forSectionController:sectionController];
}

- (UICollectionViewCell *)dequeueReusableCellWithNibName:(NSString *)nibName
Expand All @@ -1191,7 +1191,19 @@ - (UICollectionViewCell *)dequeueReusableCellWithNibName:(NSString *)nibName
UINib *nib = [UINib nibWithNibName:nibName bundle:bundle];
[collectionView registerNib:nib forCellWithReuseIdentifier:nibName];
}
return [collectionView dequeueReusableCellWithReuseIdentifier:nibName forIndexPath:indexPath];
return [self _dequeueReusableCellWithReuseIdentifier:nibName forIndexPath:indexPath forSectionController:sectionController];
}

- (UICollectionViewCell *)_dequeueReusableCellWithReuseIdentifier:(NSString *)identifier forIndexPath:(NSIndexPath *)indexPath forSectionController:(IGListSectionController *)sectionController {
// These will cause a crash in iOS 18
IGAssert(_dequeuedCells.count == 0, @"Dequeueing more than one cell (%@) for indexPath %@, section controller %@,", identifier, indexPath, sectionController);
IGAssert(_isDequeuingCell, @"Dequeueing a cell (%@) without a request from the UICollectionView for indexPath %@, section controller %@", identifier, indexPath, sectionController);

UICollectionViewCell *const cell = [self.collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath];
if (_isDequeuingCell && cell) {
[_dequeuedCells addObject:cell];
}
return cell;
}

- (__kindof UICollectionReusableView *)dequeueReusableSupplementaryViewOfKind:(NSString *)elementKind
Expand Down
13 changes: 13 additions & 0 deletions Source/IGListKit/Internal/IGListAdapter+UICollectionView.m
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cell

IGListSectionController *sectionController = [self sectionControllerForSection:indexPath.section];

#if IG_ASSERTIONS_ENABLED
if (!_dequeuedCells) {
_dequeuedCells = [NSMutableSet new];
}
#endif

// flag that a cell is being dequeued in case it tries to access a cell in the process
_isDequeuingCell = YES;
UICollectionViewCell *cell = [sectionController cellForItemAtIndex:indexPath.item];
Expand All @@ -52,8 +58,15 @@ - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cell
IGAssert(cell != nil, @"Returned a nil cell at indexPath <%@> from section controller: <%@>", indexPath, sectionController);
if (cell) {
IGAssert(cell.reuseIdentifier != nil, @"Returned a cell without a reuseIdentifier at indexPath <%@> from section controller: <%@>", indexPath, sectionController);

if (_dequeuedCells) {
// This will cause a crash in iOS 18
IGAssert([_dequeuedCells containsObject:cell], @"Returned a cell (%@) that was not dequeued at indexPath %@ from section controller %@", NSStringFromClass([cell class]), indexPath, sectionController);
}
}

[_dequeuedCells removeAllObjects];

// associate the section controller with the cell so that we know which section controller is using it
[self mapView:cell toSectionController:sectionController];

Expand Down
3 changes: 3 additions & 0 deletions Source/IGListKit/Internal/IGListAdapterInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ IGListBatchContext
>
{
__weak UICollectionView *_collectionView;

BOOL _isDequeuingCell;
NSMutableSet<UICollectionViewCell *> *_dequeuedCells;

BOOL _isDequeuingSupplementaryView;

BOOL _isSendingWorkingRangeDisplayUpdates;
Expand Down

0 comments on commit ab94446

Please sign in to comment.