From 5a93080f67ba40ab82439b90b7fbc179441c5b6b Mon Sep 17 00:00:00 2001 From: Maxime Ollivier Date: Tue, 6 Aug 2024 08:01:28 -0700 Subject: [PATCH] remove unsafe early exit on performDataSourceChange Summary: By making `-performUpdate` completely synchronous sometimes, it unearthed a bug with the early exit where multiple updates can happen at once. Specifically, the shortcut doesn't block updates from being started within `-didUpdateToObject`. Lets test removing it, since it's happening in a pretty sensitive part of updates. Differential Revision: D60773894 fbshipit-source-id: 721dffcb6d5353d12693fa873eb0f502bcdf4c02 --- Source/IGListDiffKit/IGListExperiments.h | 2 ++ Source/IGListKit/IGListAdapterUpdater.m | 4 +++- .../Internal/IGListUpdateTransactionBuilder.m | 15 ++++++++------- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Source/IGListDiffKit/IGListExperiments.h b/Source/IGListDiffKit/IGListExperiments.h index 6b75ea151..d2c3d9b90 100644 --- a/Source/IGListDiffKit/IGListExperiments.h +++ b/Source/IGListDiffKit/IGListExperiments.h @@ -20,6 +20,8 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) { IGListExperimentInvalidateLayoutForUpdates = 1 << 2, /// Throw NSInternalInconsistencyException during an update IGListExperimentThrowOnInconsistencyException = 1 << 3, + /// Remove the early exit so multiple updates can't happen at once + IGListExperimentRemoveDataSourceChangeEarlyExit = 1 << 4, }; /** diff --git a/Source/IGListKit/IGListAdapterUpdater.m b/Source/IGListKit/IGListAdapterUpdater.m index 421569edb..6f8653c0b 100644 --- a/Source/IGListKit/IGListAdapterUpdater.m +++ b/Source/IGListKit/IGListAdapterUpdater.m @@ -208,7 +208,9 @@ - (void)performDataSourceChange:(IGListDataSourceChangeBlock)block { // Which means we need to cancel the current transaction, flatten the changes from // both the current transtion and builder, and execute that new transaction. - if (!self.transaction && ![self.transactionBuilder hasChanges]) { + if (!self.transaction + && ![self.transactionBuilder hasChanges] + && !IGListExperimentEnabled(self.experiments, IGListExperimentRemoveDataSourceChangeEarlyExit)) { // If nothing is going on, lets take a shortcut. block(); return; diff --git a/Source/IGListKit/Internal/IGListUpdateTransactionBuilder.m b/Source/IGListKit/Internal/IGListUpdateTransactionBuilder.m index f11289b4e..166c29c27 100644 --- a/Source/IGListKit/Internal/IGListUpdateTransactionBuilder.m +++ b/Source/IGListKit/Internal/IGListUpdateTransactionBuilder.m @@ -141,13 +141,12 @@ - (void)addChangesFromBuilder:(IGListUpdateTransactionBuilder *)builder { - (nullable id)buildWithConfig:(IGListUpdateTransactationConfig)config delegate:(nullable id)delegate updater:(IGListAdapterUpdater *)updater { - IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock; - if (!collectionViewBlock) { - return nil; - } - switch (self.mode) { - case IGListUpdateTransactionBuilderModeBatchUpdate: + case IGListUpdateTransactionBuilderModeBatchUpdate: { + IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock; + if (!collectionViewBlock) { + return nil; + } return [[IGListBatchUpdateTransaction alloc] initWithCollectionViewBlock:collectionViewBlock updater:updater delegate:delegate @@ -157,9 +156,11 @@ - (void)addChangesFromBuilder:(IGListUpdateTransactionBuilder *)builder { applySectionDataBlock:self.applySectionDataBlock itemUpdateBlocks:self.itemUpdateBlocks completionBlocks:self.completionBlocks]; + } case IGListUpdateTransactionBuilderModeReload: { IGListReloadUpdateBlock reloadBlock = self.reloadBlock; - if (!reloadBlock) { + IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock; + if (!reloadBlock || !collectionViewBlock) { return nil; } return [[IGListReloadTransaction alloc] initWithCollectionViewBlock:collectionViewBlock