From e619edcd86f37de4ae657c22466033b5e653698a Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Sun, 27 Sep 2015 22:59:36 +0300 Subject: [PATCH 1/2] Ensure data consistency between ASDataController and its delegate while executing update transactions. --- AsyncDisplayKit/Details/ASDataController.mm | 35 ++++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index 3b80baecd0..7a6036869e 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -24,7 +24,8 @@ const static NSUInteger kASDataControllerSizingCountPerProcessor = 5; static void *kASSizingQueueContext = &kASSizingQueueContext; @interface ASDataController () { - NSMutableArray *_completedNodes; // Main thread only. External data access can immediately query this. + NSMutableArray *_externalCompletedNodes; // Main thread only. External data access can immediately query this if available. + NSMutableArray *_completedNodes; // Main thread only. External data access can immediately query this if _externalCompletedNodes is unavailable. NSMutableArray *_editingNodes; // Modified on _editingTransactionQueue only. Updates propogated to _completedNodes. NSMutableArray *_pendingEditCommandBlocks; // To be run on the main thread. Handles begin/endUpdates tracking. @@ -39,6 +40,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; @property (atomic, assign) NSUInteger batchUpdateCounter; +/// Returns nodes that can be queried externally. _externalCompletedNodes is used if available, _completedNodes otherwise. +- (NSMutableArray *)externalCompletedNodes; + @end @implementation ASDataController @@ -347,6 +351,10 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [_editingTransactionQueue addOperationWithBlock:^{ ASDisplayNodePerformBlockOnMainThread(^{ + // Deep copy _completedNodes to _externalCompletedNodes. + // Any external queries from now on will be done on _externalCompletedNodes, to guarantee data consistency with the delegate. + _externalCompletedNodes = (NSMutableArray *)ASMultidimensionalArrayDeepMutableCopy(_completedNodes); + LOG(@"endUpdatesWithCompletion - begin updates call to delegate"); [_delegate dataControllerBeginUpdates:self]; }); @@ -363,6 +371,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [_editingTransactionQueue addOperationWithBlock:^{ ASDisplayNodePerformBlockOnMainThread(^{ + // Now that the transaction is done, _completedNodes can be accessed externally again. + _externalCompletedNodes = nil; + LOG(@"endUpdatesWithCompletion - calling delegate end"); [_delegate dataController:self endUpdatesAnimated:animated completion:completion]; }); @@ -617,28 +628,31 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; - (NSUInteger)numberOfSections { ASDisplayNodeAssertMainThread(); - return [_completedNodes count]; + return [[self externalCompletedNodes] count]; } - (NSUInteger)numberOfRowsInSection:(NSUInteger)section { ASDisplayNodeAssertMainThread(); - return [_completedNodes[section] count]; + return [[self externalCompletedNodes][section] count]; } - (ASCellNode *)nodeAtIndexPath:(NSIndexPath *)indexPath { ASDisplayNodeAssertMainThread(); - return _completedNodes[indexPath.section][indexPath.row]; + return [self externalCompletedNodes][indexPath.section][indexPath.row]; } - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode; { ASDisplayNodeAssertMainThread(); + NSArray *nodes = [self externalCompletedNodes]; + NSUInteger numberOfNodes = nodes.count; + // Loop through each section to look for the cellNode - for (NSUInteger i = 0; i < [_completedNodes count]; i++) { - NSArray *sectionNodes = _completedNodes[i]; + for (NSUInteger i = 0; i < numberOfNodes; i++) { + NSArray *sectionNodes = nodes[i]; NSUInteger cellIndex = [sectionNodes indexOfObjectIdenticalTo:cellNode]; if (cellIndex != NSNotFound) { return [NSIndexPath indexPathForRow:cellIndex inSection:i]; @@ -651,13 +665,18 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; - (NSArray *)nodesAtIndexPaths:(NSArray *)indexPaths { ASDisplayNodeAssertMainThread(); - return ASFindElementsInMultidimensionalArrayAtIndexPaths(_completedNodes, [indexPaths sortedArrayUsingSelector:@selector(compare:)]); + return ASFindElementsInMultidimensionalArrayAtIndexPaths([self externalCompletedNodes], [indexPaths sortedArrayUsingSelector:@selector(compare:)]); } - (NSArray *)completedNodes { ASDisplayNodeAssertMainThread(); - return _completedNodes; + return [self externalCompletedNodes]; +} + +- (NSMutableArray *)externalCompletedNodes +{ + return _externalCompletedNodes != nil ? _externalCompletedNodes : _completedNodes; } #pragma mark - Dealloc From a94cd9dd990aa5bc7a2617c33aab8d8207f2e4c6 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Mon, 28 Sep 2015 00:18:49 +0300 Subject: [PATCH 2/2] Remove externalCompletedNodes getter in ASDataController. --- AsyncDisplayKit/Details/ASDataController.mm | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index 7a6036869e..693f1653cc 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -40,9 +40,6 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; @property (atomic, assign) NSUInteger batchUpdateCounter; -/// Returns nodes that can be queried externally. _externalCompletedNodes is used if available, _completedNodes otherwise. -- (NSMutableArray *)externalCompletedNodes; - @end @implementation ASDataController @@ -628,26 +625,26 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; - (NSUInteger)numberOfSections { ASDisplayNodeAssertMainThread(); - return [[self externalCompletedNodes] count]; + return [[self completedNodes] count]; } - (NSUInteger)numberOfRowsInSection:(NSUInteger)section { ASDisplayNodeAssertMainThread(); - return [[self externalCompletedNodes][section] count]; + return [[self completedNodes][section] count]; } - (ASCellNode *)nodeAtIndexPath:(NSIndexPath *)indexPath { ASDisplayNodeAssertMainThread(); - return [self externalCompletedNodes][indexPath.section][indexPath.row]; + return [self completedNodes][indexPath.section][indexPath.row]; } - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode; { ASDisplayNodeAssertMainThread(); - NSArray *nodes = [self externalCompletedNodes]; + NSArray *nodes = [self completedNodes]; NSUInteger numberOfNodes = nodes.count; // Loop through each section to look for the cellNode @@ -665,17 +662,13 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; - (NSArray *)nodesAtIndexPaths:(NSArray *)indexPaths { ASDisplayNodeAssertMainThread(); - return ASFindElementsInMultidimensionalArrayAtIndexPaths([self externalCompletedNodes], [indexPaths sortedArrayUsingSelector:@selector(compare:)]); + return ASFindElementsInMultidimensionalArrayAtIndexPaths((NSMutableArray *)[self completedNodes], [indexPaths sortedArrayUsingSelector:@selector(compare:)]); } +/// Returns nodes that can be queried externally. _externalCompletedNodes is used if available, _completedNodes otherwise. - (NSArray *)completedNodes { ASDisplayNodeAssertMainThread(); - return [self externalCompletedNodes]; -} - -- (NSMutableArray *)externalCompletedNodes -{ return _externalCompletedNodes != nil ? _externalCompletedNodes : _completedNodes; }