From f759d5cc7de965cc9911029c95eefbcc7a9e7551 Mon Sep 17 00:00:00 2001
From: Michael Schneider <maicki@users.noreply.github.com>
Date: Fri, 14 Sep 2018 08:48:19 -0700
Subject: [PATCH] Improve locking around clearContents (#1107)

* Improve locking around clearContents

* Add changelog
---
 CHANGELOG.md                        |  1 +
 Source/ASDisplayNode.mm             | 15 +++++++++++----
 Source/ASImageNode.mm               |  7 +++----
 Source/ASMultiplexImageNode.mm      |  5 ++++-
 Source/Details/ASRangeController.mm |  2 ++
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 731724ca30..6a9e7a5324 100644
--- 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
index c3d914671e..d84b5cbe0e 100644
--- a/Source/ASDisplayNode.mm
+++ b/Source/ASDisplayNode.mm
@@ -3144,8 +3144,10 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) {
         [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 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) {
             [[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 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) {
 - (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
index f98356d671..f30257a6b9 100644
--- a/Source/ASImageNode.mm
+++ b/Source/ASImageNode.mm
@@ -590,10 +590,9 @@ static ASDN::StaticMutex& cacheLock = *new ASDN::StaticMutex;
 - (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
index 30001e5f3f..2adc646f91 100644
--- a/Source/ASMultiplexImageNode.mm
+++ b/Source/ASMultiplexImageNode.mm
@@ -412,8 +412,11 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent
 #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
index ff0176100b..9c8b7313ce 100644
--- a/Source/Details/ASRangeController.mm
+++ b/Source/Details/ASRangeController.mm
@@ -518,6 +518,7 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive;
 // 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 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive;
 
 - (void)clearPreloadedData
 {
+  ASDisplayNodeAssertMainThread();
   for (ASCollectionElement *element in [_dataSource elementMapForRangeController:self]) {
     ASCellNode *node = element.nodeIfAllocated;
     if (ASInterfaceStateIncludesPreload(node.interfaceState)) {