Prevent data source deallocation during async updates

Summary: Saw some crashes come in from out-of-sync objects and section controllers. After some digging I found that the nil-datasource assert was firing (not very frequently). Mostly happening on other VCs, probably from hitting the back button and an update arriving. Reviewed By: jessesquires Differential Revision: D4347862 fbshipit-source-id: 38c1a4816f5c109de41297309745ac2d5e348e93
Instagram · Dec 19, 2016 · 4cc91a2 · 4cc91a2
1 parent 57c9f84
commit 4cc91a2
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,8 @@ This release closes the [2.1.0 milestone](https://github.com/Instagram/IGListKit
 
 - Avoid `UICollectionView` crashes when queueing a reload and insert/delete on the same item as well as reloading an item in a section that is animating. [Ryan Nystrom](https://github.com/rnystrom) [(#325)](https://github.com/Instagram/IGListKit/pull/325)
 
+- Prevent adapter data source from deallocating after queueing an update. [Ryan Nystrom](https://github.com/rnystrom) (tbd)
+
 2.0.0
 -----
 

diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m
@@ -113,8 +113,9 @@ - (void)setScrollViewDelegate:(id<UIScrollViewDelegate>)scrollViewDelegate {
 }
 
 - (void)updateAfterPublicSettingsChange {
-    if (_collectionView != nil && _dataSource != nil) {
-        [self updateObjects:[[_dataSource objectsForListAdapter:self] copy]];
+    id<IGListAdapterDataSource> dataSource = _dataSource;
+    if (_collectionView != nil && dataSource != nil) {
+        [self updateObjects:[[dataSource objectsForListAdapter:self] copy] dataSource:dataSource];
     }
 }
 
@@ -267,7 +268,7 @@ - (void)performUpdatesAnimated:(BOOL)animated completion:(IGListUpdaterCompletio
                                          // there are any item deletes at the same
                                          weakSelf.previoussectionMap = [weakSelf.sectionMap copy];
 
-                                         [weakSelf updateObjects:toObjects];
+                                         [weakSelf updateObjects:toObjects dataSource:dataSource];
                                      } completion:^(BOOL finished) {
                                          // release the previous items
                                          weakSelf.previoussectionMap = nil;
@@ -296,7 +297,7 @@ - (void)reloadDataWithCompletion:(nullable IGListUpdaterCompletion)completion {
     [self.updatingDelegate reloadDataWithCollectionView:collectionView reloadUpdateBlock:^{
         // purge all section controllers from the item map so that they are regenerated
         [weakSelf.sectionMap reset];
-        [weakSelf updateObjects:newItems];
+        [weakSelf updateObjects:newItems dataSource:dataSource];
     } completion:completion];
 }
 
@@ -437,7 +438,9 @@ - (CGSize)sizeForSupplementaryViewOfKind:(NSString *)elementKind atIndexPath:(NS
 
 // this method is what updates the "source of truth"
 // this should only be called just before the collection view is updated
-- (void)updateObjects:(NSArray *)objects {
+- (void)updateObjects:(NSArray *)objects dataSource:(id<IGListAdapterDataSource>)dataSource {
+    IGParameterAssert(dataSource != nil);
+
 #if DEBUG
     for (id object in objects) {
         IGAssert([object isEqual:object], @"Object instance %@ not equal to itself. This will break infra map tables.", object);
@@ -463,10 +466,10 @@ - (void)updateObjects:(NSArray *)objects {
 
         // if not, query the data source for a new one
         if (sectionController == nil) {
-            sectionController = [self.dataSource listAdapter:self sectionControllerForObject:object];
+            sectionController = [dataSource listAdapter:self sectionControllerForObject:object];
         }
 
-        IGAssert(sectionController != nil, @"Data source <%@> cannot return a nil section controller.", self.dataSource);
+        IGAssert(sectionController != nil, @"Data source <%@> cannot return a nil section controller.", dataSource);
         if (sectionController == nil) {
             continue;
         }

diff --git a/Tests/IGListAdapterE2ETests.m b/Tests/IGListAdapterE2ETests.m
@@ -1136,4 +1136,27 @@ - (void)test_whenReloadingItems_withSectionDeletedInFront_thatUpdateCanBeApplied
     [self waitForExpectationsWithTimeout:15 handler:nil];
 }
 
+- (void)test_whenDataSourceDeallocatedAfterUpdateQueued_thatUpdateSuccesfullyCompletes {
+    IGTestDelegateDataSource *dataSource = [IGTestDelegateDataSource new];
+    dataSource.objects = @[genTestObject(@1, @1)];
+    self.adapter.collectionView = self.collectionView;
+    self.adapter.dataSource = dataSource;
+    [self.collectionView layoutIfNeeded];
+
+    dataSource.objects = @[
+                           genTestObject(@1, @1),
+                           genTestObject(@2, @2),
+                           ];
+
+    XCTestExpectation *expectation = genExpectation;
+    [self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
+        XCTAssertEqual([self.collectionView numberOfSections], 2);
+        [expectation fulfill];
+    }];
+
+    dataSource = nil;
+
+    [self waitForExpectationsWithTimeout:15 handler:nil];
+}
+
 @end