From c987f668c6f1159b8ac894d4a795085f3a17c231 Mon Sep 17 00:00:00 2001 From: demolaf Date: Mon, 20 Jul 2026 15:37:48 +0100 Subject: [PATCH 1/8] fix(database): prevent FUIArray crash on desynced child events --- .../FirebaseDatabaseUITests/FUIArrayTest.m | 72 +++++++++++++++++++ FirebaseDatabaseUI/Sources/FUIArray.m | 49 ++++++------- 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m index a4797b2f657..846a5f26bff 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m @@ -485,4 +485,76 @@ - (void)testRemovesAllElementsWhenInvalidated { self.firebaseArray.count); } +#pragma mark - Invariant violations (see GitHub issue #517) + +// The local model can desync from the database (e.g. queryLimited(toLast:) plus +// concurrent server-side writes), causing a child event to reference a key that +// isn't in the array. These must not crash; each event reconciles toward the +// correct end-state instead. + +- (void)testRemovingUnknownKeyDoesNotCrash { + [self.observable populateWithCount:10]; // keys "0".."9" + self.snap.key = @"this-key-was-never-added"; + + XCTAssertNoThrow( + [self.observable sendEvent:FIRDataEventTypeChildRemoved + withObject:self.snap + previousKey:@"9" + error:nil]); + + XCTAssert(self.firebaseArray.count == 10, + @"expected count to stay 10 when removing an unknown key, got %ld", + self.firebaseArray.count); +} + +- (void)testInsertingWithUnknownPreviousKeyDoesNotCrash { + [self.observable populateWithCount:10]; // keys "0".."9" + self.snap.key = @"new"; + + XCTAssertNoThrow( + [self.observable sendEvent:FIRDataEventTypeChildAdded + withObject:self.snap + previousKey:@"this-key-was-never-added" + error:nil]); + + // Best-effort: the row is kept (appended) rather than dropped or crashing. + XCTAssert(self.firebaseArray.count == 11, + @"expected count to become 11 after insert, got %ld", + self.firebaseArray.count); + XCTAssert([[self.firebaseArray snapshotAtIndex:10].key isEqualToString:@"new"], + @"expected the new snapshot to be appended at the end"); +} + +- (void)testChangingUnknownKeyDoesNotCrash { + [self.observable populateWithCount:10]; // keys "0".."9" + self.snap.key = @"this-key-was-never-added"; + + XCTAssertNoThrow( + [self.observable sendEvent:FIRDataEventTypeChildChanged + withObject:self.snap + previousKey:@"9" + error:nil]); + + // Recovered as an insert rather than dropping the update. + XCTAssert(self.firebaseArray.count == 11, + @"expected count to become 11 after recovering change as insert, got %ld", + self.firebaseArray.count); +} + +- (void)testMovingUnknownKeyDoesNotCrash { + [self.observable populateWithCount:10]; // keys "0".."9" + self.snap.key = @"this-key-was-never-added"; + + XCTAssertNoThrow( + [self.observable sendEvent:FIRDataEventTypeChildMoved + withObject:self.snap + previousKey:@"3" + error:nil]); + + // Recovered as an insert rather than dropping the item. + XCTAssert(self.firebaseArray.count == 11, + @"expected count to become 11 after recovering move as insert, got %ld", + self.firebaseArray.count); +} + @end diff --git a/FirebaseDatabaseUI/Sources/FUIArray.m b/FirebaseDatabaseUI/Sources/FUIArray.m index 88402874261..b57b545eee8 100755 --- a/FirebaseDatabaseUI/Sources/FUIArray.m +++ b/FirebaseDatabaseUI/Sources/FUIArray.m @@ -189,20 +189,18 @@ - (NSUInteger)indexForKey:(NSString *)key { } - (void)insertSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)previous { + // The local model can desync from the database (e.g. a query limited via + // -queryLimitedToLast: combined with concurrent server-side writes), so a + // child event may reference a key that isn't in the array. Reconcile toward + // the correct end-state instead of throwing. See GitHub issue #517. NSUInteger index = 0; if (previous != nil) { - NSInteger previousChildIndex = (NSInteger)[self indexForKey:previous]; - - if (previousChildIndex == NSNotFound) { - NSString *reason = [NSString stringWithFormat:@"Attempted to insert snapshot with unknown" - @" previousChildKey %@ into array: %@", previous, self.snapshots]; - NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException - reason:reason - userInfo:nil]; - @throw exception; - } + NSUInteger previousChildIndex = [self indexForKey:previous]; - index = previousChildIndex + 1; + // The previous sibling was never delivered locally. Append rather than + // crash or drop the row. + index = (previousChildIndex == NSNotFound) ? self.snapshots.count + : previousChildIndex + 1; } [self.snapshots insertObject:snap atIndex:index]; @@ -217,12 +215,9 @@ - (void)removeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *) NSUInteger index = [self indexForKey:snap.key]; if (index == NSNotFound) { - NSString *reason = [NSString stringWithFormat:@"Attempted to remove snapshot with unknown" - @" key %@ from array: %@", snap.key, self.snapshots]; - NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException - reason:reason - userInfo:nil]; - @throw exception; + // Already absent from the local model; the desired end-state (item gone) + // already holds, so there is nothing to remove. See GitHub issue #517. + return; } [self.snapshots removeObjectAtIndex:index]; @@ -237,12 +232,10 @@ - (void)changeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *) NSUInteger index = [self indexForKey:snap.key]; if (index == NSNotFound) { - NSString *reason = [NSString stringWithFormat:@"Attempted to replace snapshot with unknown" - @" key %@ in array: %@", snap.key, self.snapshots]; - NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException - reason:reason - userInfo:nil]; - @throw exception; + // We never had this item; recover it as an insert rather than dropping the + // update. See GitHub issue #517. + [self insertSnapshot:snap withPreviousChildKey:previous]; + return; } [self.snapshots replaceObjectAtIndex:index withObject:snap]; @@ -257,12 +250,10 @@ - (void)moveSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)pr NSUInteger fromIndex = [self indexForKey:snap.key]; if (fromIndex == NSNotFound) { - NSString *reason = [NSString stringWithFormat:@"Attempted to remove snapshot with unknown" - @" key %@ from array: %@", snap.key, self.snapshots]; - NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException - reason:reason - userInfo:nil]; - @throw exception; + // The item we were asked to move isn't in the local model; recover it as an + // insert at the destination rather than dropping it. See GitHub issue #517. + [self insertSnapshot:snap withPreviousChildKey:previous]; + return; } [self.snapshots removeObjectAtIndex:fromIndex]; From 06d386f17fd96a8c9759fe393919fcc99bda4a9f Mon Sep 17 00:00:00 2001 From: demolaf Date: Mon, 20 Jul 2026 15:43:35 +0100 Subject: [PATCH 2/8] remove unnecessary comments --- .../FirebaseDatabaseUITests/FUIArrayTest.m | 18 +++++------------- FirebaseDatabaseUI/Sources/FUIArray.m | 13 ------------- 2 files changed, 5 insertions(+), 26 deletions(-) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m index 846a5f26bff..93e453442e8 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m @@ -485,15 +485,10 @@ - (void)testRemovesAllElementsWhenInvalidated { self.firebaseArray.count); } -#pragma mark - Invariant violations (see GitHub issue #517) - -// The local model can desync from the database (e.g. queryLimited(toLast:) plus -// concurrent server-side writes), causing a child event to reference a key that -// isn't in the array. These must not crash; each event reconciles toward the -// correct end-state instead. +#pragma mark - Invariant violations - (void)testRemovingUnknownKeyDoesNotCrash { - [self.observable populateWithCount:10]; // keys "0".."9" + [self.observable populateWithCount:10]; self.snap.key = @"this-key-was-never-added"; XCTAssertNoThrow( @@ -508,7 +503,7 @@ - (void)testRemovingUnknownKeyDoesNotCrash { } - (void)testInsertingWithUnknownPreviousKeyDoesNotCrash { - [self.observable populateWithCount:10]; // keys "0".."9" + [self.observable populateWithCount:10]; self.snap.key = @"new"; XCTAssertNoThrow( @@ -517,7 +512,6 @@ - (void)testInsertingWithUnknownPreviousKeyDoesNotCrash { previousKey:@"this-key-was-never-added" error:nil]); - // Best-effort: the row is kept (appended) rather than dropped or crashing. XCTAssert(self.firebaseArray.count == 11, @"expected count to become 11 after insert, got %ld", self.firebaseArray.count); @@ -526,7 +520,7 @@ - (void)testInsertingWithUnknownPreviousKeyDoesNotCrash { } - (void)testChangingUnknownKeyDoesNotCrash { - [self.observable populateWithCount:10]; // keys "0".."9" + [self.observable populateWithCount:10]; self.snap.key = @"this-key-was-never-added"; XCTAssertNoThrow( @@ -535,14 +529,13 @@ - (void)testChangingUnknownKeyDoesNotCrash { previousKey:@"9" error:nil]); - // Recovered as an insert rather than dropping the update. XCTAssert(self.firebaseArray.count == 11, @"expected count to become 11 after recovering change as insert, got %ld", self.firebaseArray.count); } - (void)testMovingUnknownKeyDoesNotCrash { - [self.observable populateWithCount:10]; // keys "0".."9" + [self.observable populateWithCount:10]; self.snap.key = @"this-key-was-never-added"; XCTAssertNoThrow( @@ -551,7 +544,6 @@ - (void)testMovingUnknownKeyDoesNotCrash { previousKey:@"3" error:nil]); - // Recovered as an insert rather than dropping the item. XCTAssert(self.firebaseArray.count == 11, @"expected count to become 11 after recovering move as insert, got %ld", self.firebaseArray.count); diff --git a/FirebaseDatabaseUI/Sources/FUIArray.m b/FirebaseDatabaseUI/Sources/FUIArray.m index b57b545eee8..4df110c6779 100755 --- a/FirebaseDatabaseUI/Sources/FUIArray.m +++ b/FirebaseDatabaseUI/Sources/FUIArray.m @@ -189,16 +189,9 @@ - (NSUInteger)indexForKey:(NSString *)key { } - (void)insertSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)previous { - // The local model can desync from the database (e.g. a query limited via - // -queryLimitedToLast: combined with concurrent server-side writes), so a - // child event may reference a key that isn't in the array. Reconcile toward - // the correct end-state instead of throwing. See GitHub issue #517. NSUInteger index = 0; if (previous != nil) { NSUInteger previousChildIndex = [self indexForKey:previous]; - - // The previous sibling was never delivered locally. Append rather than - // crash or drop the row. index = (previousChildIndex == NSNotFound) ? self.snapshots.count : previousChildIndex + 1; } @@ -215,8 +208,6 @@ - (void)removeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *) NSUInteger index = [self indexForKey:snap.key]; if (index == NSNotFound) { - // Already absent from the local model; the desired end-state (item gone) - // already holds, so there is nothing to remove. See GitHub issue #517. return; } @@ -232,8 +223,6 @@ - (void)changeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *) NSUInteger index = [self indexForKey:snap.key]; if (index == NSNotFound) { - // We never had this item; recover it as an insert rather than dropping the - // update. See GitHub issue #517. [self insertSnapshot:snap withPreviousChildKey:previous]; return; } @@ -250,8 +239,6 @@ - (void)moveSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)pr NSUInteger fromIndex = [self indexForKey:snap.key]; if (fromIndex == NSNotFound) { - // The item we were asked to move isn't in the local model; recover it as an - // insert at the destination rather than dropping it. See GitHub issue #517. [self insertSnapshot:snap withPreviousChildKey:previous]; return; } From d21c655e48420f1bd610ea8a9b73aefda4fad6a4 Mon Sep 17 00:00:00 2001 From: demolaf Date: Mon, 20 Jul 2026 22:29:18 +0100 Subject: [PATCH 3/8] fix(sample): avoid force-unwrap crash on malformed chat snapshot --- .../Samples/Chat/ChatViewController.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/samples/swift/FirebaseUI-demo-swift/Samples/Chat/ChatViewController.swift b/samples/swift/FirebaseUI-demo-swift/Samples/Chat/ChatViewController.swift index 9ce84c724f8..9efb28a0212 100644 --- a/samples/swift/FirebaseUI-demo-swift/Samples/Chat/ChatViewController.swift +++ b/samples/swift/FirebaseUI-demo-swift/Samples/Chat/ChatViewController.swift @@ -64,7 +64,7 @@ class ChatViewController: UIViewController, UICollectionViewDelegateFlowLayout { self.collectionView.bind(to: self.query!) { (view, indexPath, snap) -> UICollectionViewCell in let cell = view.dequeueReusableCell(withReuseIdentifier: ChatViewController.reuseIdentifier, for: indexPath) as! ChatCollectionViewCell - let chat = Chat(snapshot: snap)! + guard let chat = Chat(snapshot: snap) else { return cell } cell.populateCellWithChat(chat, user: self.user, maxWidth: self.view.frame.size.width) return cell } @@ -184,7 +184,9 @@ class ChatViewController: UIViewController, UICollectionViewDelegateFlowLayout { let width = self.view.frame.size.width let blob = self.collectionViewDataSource.snapshot(at: indexPath.item) - let text = Chat(snapshot: blob)!.text + guard let text = Chat(snapshot: blob)?.text else { + return CGSize(width: width, height: 0) + } let rect = ChatCollectionViewCell.boundingRectForText(text, maxWidth: width) From f13769be31c14f78dabc374ad3d66f76bb363d7c Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 00:45:46 +0100 Subject: [PATCH 4/8] fix(database): serialize FUICollectionViewDataSource batch updates --- .../Sources/FUICollectionViewDataSource.m | 55 +++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m index b20874dc687..a14d1f6db19 100644 --- a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m +++ b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m @@ -38,6 +38,10 @@ @interface FUICollectionViewDataSource () @property (strong, nonatomic, readonly) UICollectionViewCell *(^populateCellAtIndexPath) (UICollectionView *collectionView, NSIndexPath *indexPath, FIRDataSnapshot *object); +@property (nonatomic, strong) NSMutableArray *pendingUpdates; + +@property (nonatomic, assign) BOOL isApplyingBatchUpdate; + @end @implementation FUICollectionViewDataSource @@ -96,30 +100,34 @@ - (void)unbind { // performBatchUpdates: is used for single updates because of this radar: // https://openradar.appspot.com/26484150 - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)index { - [self.collectionView performBatchUpdates:^{ - self.count = array.count; + self.count = array.count; + [self enqueueUpdate:^{ [self.collectionView insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; - } completion:^(BOOL finished) {}]; + }]; } - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)index { - [self.collectionView - reloadItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + [self enqueueUpdate:^{ + [self.collectionView + reloadItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + }]; } - (void)array:(FUIArray *)array didRemoveObject:(id)object atIndex:(NSUInteger)index { - [self.collectionView performBatchUpdates:^{ - self.count = array.count; + self.count = array.count; + [self enqueueUpdate:^{ [self.collectionView deleteItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; - } completion:^(BOOL finished) {}]; + }]; } - (void)array:(FUIArray *)array didMoveObject:(id)object fromIndex:(NSUInteger)fromIndex toIndex:(NSUInteger)toIndex { - [self.collectionView moveItemAtIndexPath:[NSIndexPath indexPathForItem:fromIndex inSection:0] - toIndexPath:[NSIndexPath indexPathForItem:toIndex inSection:0]]; + [self enqueueUpdate:^{ + [self.collectionView moveItemAtIndexPath:[NSIndexPath indexPathForItem:fromIndex inSection:0] + toIndexPath:[NSIndexPath indexPathForItem:toIndex inSection:0]]; + }]; } - (void)array:(id)array queryCancelledWithError:(NSError *)error { @@ -128,6 +136,33 @@ - (void)array:(id)array queryCancelledWithError:(NSError *)error } } +- (void)enqueueUpdate:(dispatch_block_t)update { + if (self.pendingUpdates == nil) { + self.pendingUpdates = [NSMutableArray array]; + } + [self.pendingUpdates addObject:update]; + [self flushPendingUpdatesIfNeeded]; +} + +- (void)flushPendingUpdatesIfNeeded { + if (self.isApplyingBatchUpdate || self.pendingUpdates.count == 0) { + return; + } + + NSArray *batch = self.pendingUpdates; + self.pendingUpdates = [NSMutableArray array]; + self.isApplyingBatchUpdate = YES; + + [self.collectionView performBatchUpdates:^{ + for (dispatch_block_t update in batch) { + update(); + } + } completion:^(BOOL finished) { + self.isApplyingBatchUpdate = NO; + [self flushPendingUpdatesIfNeeded]; + }]; +} + #pragma mark - UICollectionViewDataSource methods - (nonnull UICollectionViewCell *)collectionView:(nonnull UICollectionView *)collectionView From eeb87c86d40a4500a9609697421acedb7dd38444 Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 09:29:41 +0100 Subject: [PATCH 5/8] fix(database): keep one op per performBatchUpdates call in FUICollectionViewDataSource --- .../FUICollectionViewDataSourceTest.m | 4 ++ .../Sources/FUICollectionViewDataSource.m | 66 ++++++++++++------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m index d69838f7a42..1e8920db7e5 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m @@ -69,6 +69,10 @@ - (void)tearDown { } - (void)testItHasACount { + NSDate *timeout = [NSDate dateWithTimeIntervalSinceNow:1.0]; + while (self.dataSource.count != 10 && timeout.timeIntervalSinceNow > 0) { + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.01]]; + } NSUInteger count = self.dataSource.count; XCTAssert(count == 10, @"expected data source to have 10 elements after 10 insertions, but got %lu", count); } diff --git a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m index a14d1f6db19..85c0a2d70e3 100644 --- a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m +++ b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m @@ -38,7 +38,7 @@ @interface FUICollectionViewDataSource () @property (strong, nonatomic, readonly) UICollectionViewCell *(^populateCellAtIndexPath) (UICollectionView *collectionView, NSIndexPath *indexPath, FIRDataSnapshot *object); -@property (nonatomic, strong) NSMutableArray *pendingUpdates; +@property (nonatomic, strong) NSMutableArray *pendingUpdates; @property (nonatomic, assign) BOOL isApplyingBatchUpdate; @@ -100,33 +100,51 @@ - (void)unbind { // performBatchUpdates: is used for single updates because of this radar: // https://openradar.appspot.com/26484150 - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)index { - self.count = array.count; - [self enqueueUpdate:^{ - [self.collectionView - insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + NSUInteger newCount = array.count; + [self enqueueUpdate:^(dispatch_block_t done) { + [self.collectionView performBatchUpdates:^{ + self.count = newCount; + [self.collectionView + insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + } completion:^(BOOL finished) { + done(); + }]; }]; } - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)index { - [self enqueueUpdate:^{ - [self.collectionView - reloadItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + [self enqueueUpdate:^(dispatch_block_t done) { + [self.collectionView performBatchUpdates:^{ + [self.collectionView + reloadItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + } completion:^(BOOL finished) { + done(); + }]; }]; } - (void)array:(FUIArray *)array didRemoveObject:(id)object atIndex:(NSUInteger)index { - self.count = array.count; - [self enqueueUpdate:^{ - [self.collectionView - deleteItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + NSUInteger newCount = array.count; + [self enqueueUpdate:^(dispatch_block_t done) { + [self.collectionView performBatchUpdates:^{ + self.count = newCount; + [self.collectionView + deleteItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; + } completion:^(BOOL finished) { + done(); + }]; }]; } - (void)array:(FUIArray *)array didMoveObject:(id)object fromIndex:(NSUInteger)fromIndex toIndex:(NSUInteger)toIndex { - [self enqueueUpdate:^{ - [self.collectionView moveItemAtIndexPath:[NSIndexPath indexPathForItem:fromIndex inSection:0] - toIndexPath:[NSIndexPath indexPathForItem:toIndex inSection:0]]; + [self enqueueUpdate:^(dispatch_block_t done) { + [self.collectionView performBatchUpdates:^{ + [self.collectionView moveItemAtIndexPath:[NSIndexPath indexPathForItem:fromIndex inSection:0] + toIndexPath:[NSIndexPath indexPathForItem:toIndex inSection:0]]; + } completion:^(BOOL finished) { + done(); + }]; }]; } @@ -136,7 +154,7 @@ - (void)array:(id)array queryCancelledWithError:(NSError *)error } } -- (void)enqueueUpdate:(dispatch_block_t)update { +- (void)enqueueUpdate:(void (^)(dispatch_block_t done))update { if (self.pendingUpdates == nil) { self.pendingUpdates = [NSMutableArray array]; } @@ -149,18 +167,16 @@ - (void)flushPendingUpdatesIfNeeded { return; } - NSArray *batch = self.pendingUpdates; - self.pendingUpdates = [NSMutableArray array]; + void (^next)(dispatch_block_t) = self.pendingUpdates.firstObject; + [self.pendingUpdates removeObjectAtIndex:0]; self.isApplyingBatchUpdate = YES; - [self.collectionView performBatchUpdates:^{ - for (dispatch_block_t update in batch) { - update(); - } - } completion:^(BOOL finished) { + next(^{ self.isApplyingBatchUpdate = NO; - [self flushPendingUpdatesIfNeeded]; - }]; + dispatch_async(dispatch_get_main_queue(), ^{ + [self flushPendingUpdatesIfNeeded]; + }); + }); } #pragma mark - UICollectionViewDataSource methods From 19dcebdcdd58c8f3631f36bd83d9a76e7d054f5f Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 10:06:23 +0100 Subject: [PATCH 6/8] fix(database): keep FUICollectionViewDataSource items in lockstep with applied UI updates --- .../FUICollectionViewDataSourceTest.m | 8 ++++++-- .../Sources/FUICollectionViewDataSource.m | 20 ++++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m index 1e8920db7e5..5b72f0ff603 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m @@ -59,6 +59,7 @@ - (void)setUp { NSLog(@"%lu", (unsigned long)[self.collectionView numberOfItemsInSection:0]); [self.observable populateWithCount:10]; + [self waitForDataSourceCount:10]; } - (void)tearDown { @@ -68,11 +69,14 @@ - (void)tearDown { [super tearDown]; } -- (void)testItHasACount { +- (void)waitForDataSourceCount:(NSUInteger)expected { NSDate *timeout = [NSDate dateWithTimeIntervalSinceNow:1.0]; - while (self.dataSource.count != 10 && timeout.timeIntervalSinceNow > 0) { + while (self.dataSource.count != expected && timeout.timeIntervalSinceNow > 0) { [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.01]]; } +} + +- (void)testItHasACount { NSUInteger count = self.dataSource.count; XCTAssert(count == 10, @"expected data source to have 10 elements after 10 insertions, but got %lu", count); } diff --git a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m index 85c0a2d70e3..4823fc6f09f 100644 --- a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m +++ b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m @@ -42,6 +42,8 @@ @interface FUICollectionViewDataSource () @property (nonatomic, assign) BOOL isApplyingBatchUpdate; +@property (nonatomic, strong) NSMutableArray *displayedSnapshots; + @end @implementation FUICollectionViewDataSource @@ -59,6 +61,7 @@ - (instancetype)initWithCollection:(id)collection _populateCellAtIndexPath = populateCell; _count = 0; // This is zero because RTDB arrays start out at zero // and send initial items as a series of adds. + _displayedSnapshots = [NSMutableArray array]; } return self; } @@ -76,11 +79,11 @@ - (NSUInteger)count { } - (NSArray *)items { - return self.collection.items; + return [self.displayedSnapshots copy]; } - (FIRDataSnapshot *)snapshotAtIndex:(NSInteger)index { - return [self.collection snapshotAtIndex:index]; + return self.displayedSnapshots[index]; } - (void)bindToView:(UICollectionView *)view { @@ -100,10 +103,10 @@ - (void)unbind { // performBatchUpdates: is used for single updates because of this radar: // https://openradar.appspot.com/26484150 - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)index { - NSUInteger newCount = array.count; [self enqueueUpdate:^(dispatch_block_t done) { [self.collectionView performBatchUpdates:^{ - self.count = newCount; + [self.displayedSnapshots insertObject:object atIndex:index]; + self.count = self.displayedSnapshots.count; [self.collectionView insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; } completion:^(BOOL finished) { @@ -115,6 +118,7 @@ - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)inde - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)index { [self enqueueUpdate:^(dispatch_block_t done) { [self.collectionView performBatchUpdates:^{ + self.displayedSnapshots[index] = object; [self.collectionView reloadItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; } completion:^(BOOL finished) { @@ -124,10 +128,10 @@ - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)i } - (void)array:(FUIArray *)array didRemoveObject:(id)object atIndex:(NSUInteger)index { - NSUInteger newCount = array.count; [self enqueueUpdate:^(dispatch_block_t done) { [self.collectionView performBatchUpdates:^{ - self.count = newCount; + [self.displayedSnapshots removeObjectAtIndex:index]; + self.count = self.displayedSnapshots.count; [self.collectionView deleteItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:index inSection:0] ]]; } completion:^(BOOL finished) { @@ -140,6 +144,8 @@ - (void)array:(FUIArray *)array didMoveObject:(id)object fromIndex:(NSUInteger)fromIndex toIndex:(NSUInteger)toIndex { [self enqueueUpdate:^(dispatch_block_t done) { [self.collectionView performBatchUpdates:^{ + [self.displayedSnapshots removeObjectAtIndex:fromIndex]; + [self.displayedSnapshots insertObject:object atIndex:toIndex]; [self.collectionView moveItemAtIndexPath:[NSIndexPath indexPathForItem:fromIndex inSection:0] toIndexPath:[NSIndexPath indexPathForItem:toIndex inSection:0]]; } completion:^(BOOL finished) { @@ -183,7 +189,7 @@ - (void)flushPendingUpdatesIfNeeded { - (nonnull UICollectionViewCell *)collectionView:(nonnull UICollectionView *)collectionView cellForItemAtIndexPath:(nonnull NSIndexPath *)indexPath { - FIRDataSnapshot *snap = [self.collection.items objectAtIndex:indexPath.item]; + FIRDataSnapshot *snap = self.displayedSnapshots[indexPath.item]; UICollectionViewCell *cell = self.populateCellAtIndexPath(collectionView, indexPath, snap); From 17cf5694e0379686e5349af7fc5fd1f814a0f01f Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 11:56:38 +0100 Subject: [PATCH 7/8] test(database): assert recovered-insert index per gemini-code-assist review --- FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m index 93e453442e8..699db2e4def 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m @@ -532,6 +532,8 @@ - (void)testChangingUnknownKeyDoesNotCrash { XCTAssert(self.firebaseArray.count == 11, @"expected count to become 11 after recovering change as insert, got %ld", self.firebaseArray.count); + XCTAssert([[self.firebaseArray snapshotAtIndex:10].key isEqualToString:@"this-key-was-never-added"], + @"expected the recovered snapshot to be inserted at index 10"); } - (void)testMovingUnknownKeyDoesNotCrash { @@ -547,6 +549,8 @@ - (void)testMovingUnknownKeyDoesNotCrash { XCTAssert(self.firebaseArray.count == 11, @"expected count to become 11 after recovering move as insert, got %ld", self.firebaseArray.count); + XCTAssert([[self.firebaseArray snapshotAtIndex:4].key isEqualToString:@"this-key-was-never-added"], + @"expected the recovered snapshot to be inserted at index 4"); } @end From 63ec717ed41883e006017bc45f804fe6eddbd874 Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 13:00:25 +0100 Subject: [PATCH 8/8] fix(database): guard FUICollectionViewDataSource queue against nil collectionView on unbind --- .../FUICollectionViewDataSourceTest.m | 15 +++++++++++++++ .../Sources/FUICollectionViewDataSource.m | 16 ++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m index 5b72f0ff603..ad8034f8876 100644 --- a/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m +++ b/FirebaseDatabaseUI/FirebaseDatabaseUITests/FUICollectionViewDataSourceTest.m @@ -127,4 +127,19 @@ - (void)testItMovesCells { found %@ at index 4 instead", cell.accessibilityValue); } +- (void)testUnbindDoesNotLeakDataSourceWithPendingItems { + __weak FUICollectionViewDataSource *weakDataSource = self.dataSource; + + [self.dataSource unbind]; + self.dataSource = nil; + + NSDate *timeout = [NSDate dateWithTimeIntervalSinceNow:1.0]; + while (weakDataSource != nil && timeout.timeIntervalSinceNow > 0) { + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.01]]; + } + + XCTAssertNil(weakDataSource, + @"expected data source to be deallocated after unbind with pending items, found a retain cycle"); +} + @end diff --git a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m index 4823fc6f09f..83c0a344075 100644 --- a/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m +++ b/FirebaseDatabaseUI/Sources/FUICollectionViewDataSource.m @@ -104,6 +104,10 @@ - (void)unbind { // https://openradar.appspot.com/26484150 - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)index { [self enqueueUpdate:^(dispatch_block_t done) { + if (self.collectionView == nil) { + done(); + return; + } [self.collectionView performBatchUpdates:^{ [self.displayedSnapshots insertObject:object atIndex:index]; self.count = self.displayedSnapshots.count; @@ -117,6 +121,10 @@ - (void)array:(FUIArray *)array didAddObject:(id)object atIndex:(NSUInteger)inde - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)index { [self enqueueUpdate:^(dispatch_block_t done) { + if (self.collectionView == nil) { + done(); + return; + } [self.collectionView performBatchUpdates:^{ self.displayedSnapshots[index] = object; [self.collectionView @@ -129,6 +137,10 @@ - (void)array:(FUIArray *)array didChangeObject:(id)object atIndex:(NSUInteger)i - (void)array:(FUIArray *)array didRemoveObject:(id)object atIndex:(NSUInteger)index { [self enqueueUpdate:^(dispatch_block_t done) { + if (self.collectionView == nil) { + done(); + return; + } [self.collectionView performBatchUpdates:^{ [self.displayedSnapshots removeObjectAtIndex:index]; self.count = self.displayedSnapshots.count; @@ -143,6 +155,10 @@ - (void)array:(FUIArray *)array didRemoveObject:(id)object atIndex:(NSUInteger)i - (void)array:(FUIArray *)array didMoveObject:(id)object fromIndex:(NSUInteger)fromIndex toIndex:(NSUInteger)toIndex { [self enqueueUpdate:^(dispatch_block_t done) { + if (self.collectionView == nil) { + done(); + return; + } [self.collectionView performBatchUpdates:^{ [self.displayedSnapshots removeObjectAtIndex:fromIndex]; [self.displayedSnapshots insertObject:object atIndex:toIndex];