Skip to content

Commit

Permalink
Only clear ASCollectionView's data during deallocation
Browse files Browse the repository at this point in the history
This is a follow up on TextureGroup#1136. Our experiment results show that clearing data frequently is the cause of our TextureGroup#1 crash. @maicki and I believe that this is because if the collection view is being used, silently clearing its data without notifying the backing UICollectionView can put it out-of-sync and causes mayhem next time the collection view processes a batch update. If you look at the stack trace closely, you'll notice that the crash doesn't occur on the same run loop that clearData is called. This made it extremely tricky to investigate and identify the root cause.

Another interesting question would be whether or not we want to clear the data during deallocation at all, since the data will be cleared out soon anyway.
  • Loading branch information
nguyenhuy committed Oct 1, 2018
1 parent 5e05793 commit 93312f2
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ - (void)_asyncDelegateOrDataSourceDidChange
{
ASDisplayNodeAssertMainThread();

if (_asyncDataSource == nil && _asyncDelegate == nil && !ASActivateExperimentalFeature(ASExperimentalSkipClearData)) {
if (_asyncDataSource == nil && _asyncDelegate == nil && _isDeallocating && ASActivateExperimentalFeature(ASExperimentalClearDataDuringDeallocation)) {
[_dataController clearData];
}
}
Expand Down
7 changes: 4 additions & 3 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalUnfairLock = 1 << 3, // exp_unfair_lock
ASExperimentalLayerDefaults = 1 << 4, // exp_infer_layer_defaults
ASExperimentalNetworkImageQueue = 1 << 5, // exp_network_image_queue
ASExperimentalCollectionTeardown = 1 << 6, // exp_collection_teardown
ASExperimentalFramesetterCache = 1 << 7, // exp_framesetter_cache
ASExperimentalSkipClearData = 1 << 8, // exp_skip_clear_data
ASExperimentalDeallocQueue = 1 << 6, // exp_dealloc_queue_v2
ASExperimentalCollectionTeardown = 1 << 7, // exp_collection_teardown
ASExperimentalFramesetterCache = 1 << 8, // exp_framesetter_cache
ASExperimentalClearDataDuringDeallocation = 1 << 9, // exp_clear_data_during_deallocation
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down

0 comments on commit 93312f2

Please sign in to comment.