Improve locking around clearContents (#1107)

* Improve locking around clearContents * Add changelog
TextureGroup · Sep 14, 2018 · f759d5c · f759d5c
1 parent eb5bd09
commit f759d5c
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@
 - Remove double scaling of lineHeightMultiple & paragraphSpacing attributes in ASTextKitFontSizeAdjuster. [Eric Jensen](https://github.com/ejensen)
 - Add a delegate callback for when the framework has initialized. [Adlai Holler](https://github.com/Adlai-Holler)
 - Improve TextNode2 by skipping an unneeded copy during measurement. [Adlai Holler](https://github.com/Adlai-Holler)
+- Improve locking around clearContents [Michael Schneider](https://github.com/maicki)
 
 ## 2.7
 - Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)

diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -3144,8 +3144,10 @@ - (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState
         [self setDisplaySuspended:YES];
         //schedule clear contents on next runloop
         dispatch_async(dispatch_get_main_queue(), ^{
-          ASDN::MutexLocker l(__instanceLock__);
-          if (ASInterfaceStateIncludesDisplay(_interfaceState) == NO) {
+          __instanceLock__.lock();
+          ASInterfaceState interfaceState = _interfaceState;
+          __instanceLock__.unlock();
+          if (ASInterfaceStateIncludesDisplay(interfaceState) == NO) {
             [self clearContents];
           }
         });
@@ -3162,8 +3164,10 @@ - (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState
             [[self asyncLayer] cancelAsyncDisplay];
             //schedule clear contents on next runloop
             dispatch_async(dispatch_get_main_queue(), ^{
-              ASDN::MutexLocker l(__instanceLock__);
-              if (ASInterfaceStateIncludesDisplay(_interfaceState) == NO) {
+              __instanceLock__.lock();
+              ASInterfaceState interfaceState = _interfaceState;
+              __instanceLock__.unlock();
+              if (ASInterfaceStateIncludesDisplay(interfaceState) == NO) {
                 [self clearContents];
               }
             });
@@ -3355,6 +3359,9 @@ - (void)didExitPreloadState
 - (void)clearContents
 {
   ASDisplayNodeAssertMainThread();
+  ASAssertUnlocked(__instanceLock__);
+
+  ASDN::MutexLocker l(__instanceLock__);
   if (_flags.canClearContentsOfLayer) {
     // No-op if these haven't been created yet, as that guarantees they don't have contents that needs to be released.
     _layer.contents = nil;

diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm
@@ -590,10 +590,9 @@ - (void)setNeedsDisplayWithCompletion:(void (^ _Nullable)(BOOL canceled))display
 - (void)clearContents
 {
   [super clearContents];
-
-  __instanceLock__.lock();
-    _weakCacheEntry = nil;  // release contents from the cache.
-  __instanceLock__.unlock();
+
+  ASDN::MutexLocker l(__instanceLock__);
+  _weakCacheEntry = nil;  // release contents from the cache.
 }
 
 #pragma mark - Cropping

diff --git a/Source/ASMultiplexImageNode.mm b/Source/ASMultiplexImageNode.mm
@@ -412,8 +412,11 @@ - (void)reloadImageIdentifierSources
 #pragma mark - Core Internal
 - (void)_setDisplayedImageIdentifier:(id)displayedImageIdentifier withImage:(UIImage *)image
 {
-  if (ASObjectIsEqual(displayedImageIdentifier, _displayedImageIdentifier))
+  ASDisplayNodeAssertMainThread();
+
+  if (ASObjectIsEqual(_displayedImageIdentifier, displayedImageIdentifier)) {
     return;
+  }
 
   _displayedImageIdentifier = displayedImageIdentifier;
 

diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm
@@ -518,6 +518,7 @@ - (void)dataController:(ASDataController *)dataController updateWithChangeSet:(_
 // Skip the many method calls of the recursive operation if the top level cell node already has the right interfaceState.
 - (void)clearContents
 {
+  ASDisplayNodeAssertMainThread();
   for (ASCollectionElement *element in [_dataSource elementMapForRangeController:self]) {
     ASCellNode *node = element.nodeIfAllocated;
     if (ASInterfaceStateIncludesDisplay(node.interfaceState)) {
@@ -528,6 +529,7 @@ - (void)clearContents
 
 - (void)clearPreloadedData
 {
+  ASDisplayNodeAssertMainThread();
   for (ASCollectionElement *element in [_dataSource elementMapForRangeController:self]) {
     ASCellNode *node = element.nodeIfAllocated;
     if (ASInterfaceStateIncludesPreload(node.interfaceState)) {