Fix a layout deadlock caused by holding the lock and going up the tre…

…e. (#638)

* Fix a layout deadlock caused by holding the lock and going up the tree.

* Add CHANGELOG message

* Huy's comments

CHANGELOG.md

@@ -5,6 +5,7 @@
 - [Animated Image] Adds support for animated WebP as well as improves GIF handling. [#605](https://github.com/TextureGroup/Texture/pull/605) [Garrett Moon](https://github.com/garrettmoon)
 - [ASCollectionView] Check if batch fetching is needed if batch fetching parameter has been changed. [#624](https://github.com/TextureGroup/Texture/pull/624) [Garrett Moon](https://github.com/garrettmoon)
 - [ASNetworkImageNode] New delegate callback to tell the consumer whether the image was loaded from cache or download. [Adlai Holler](https://github.com/Adlai-Holler)
+- [Layout] Fixes a deadlock in layout. [#638](https://github.com/TextureGroup/Texture/pull/638) [Garrett Moon](https://github.com/garrettmoon)
 
 ## 2.6
 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)

Source/ASDisplayNode+Layout.mm

@@ -211,8 +211,9 @@ @implementation ASDisplayNode (ASLayoutInternal)
  * @discussion The size of a root node is determined by each subnode. Calling invalidateSize will let the root node know
  * that the intrinsic size of the receiver node is no longer valid and a resizing of the root node needs to happen.
  */
-- (void)_setNeedsLayoutFromAbove
+- (void)_u_setNeedsLayoutFromAbove
 {
+  ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock);
   as_activity_create_for_scope("Set needs layout from above");
   ASDisplayNodeAssertThreadAffinity(self);
 
@@ -227,7 +228,7 @@ - (void)_setNeedsLayoutFromAbove
 
   if (supernode) {
     // Threading model requires that we unlock before calling a method on our parent.
-    [supernode _setNeedsLayoutFromAbove];
+    [supernode _u_setNeedsLayoutFromAbove];
   } else {
     // Let the root node method know that the size was invalidated
     [self _rootNodeDidInvalidateSize];
@@ -287,8 +288,10 @@ - (void)displayNodeDidInvalidateSizeNewSize:(CGSize)size
   }
 }
 
-- (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
+- (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds
 {
+  ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock);
+  ASDN::MutexLocker l(__instanceLock__);
   // Check if we are a subnode in a layout transition.
   // In this case no measurement is needed as it's part of the layout transition
   if ([self _isLayoutTransitionInvalid]) {
@@ -368,8 +371,10 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
     // In this case, we need to detect that we've already asked to be resized to match this
     // particular ASLayout object, and shouldn't loop asking again unless we have a different ASLayout.
     nextLayout->requestedLayoutFromAbove = YES;
-    [self _setNeedsLayoutFromAbove];
-    // Update the layout's version here because _setNeedsLayoutFromAbove calls __setNeedsLayout which in turn increases _layoutVersion
+    __instanceLock__.unlock();
+    [self _u_setNeedsLayoutFromAbove];
+    __instanceLock__.lock();
+    // Update the layout's version here because _u_setNeedsLayoutFromAbove calls __setNeedsLayout which in turn increases _layoutVersion
     // Failing to do this will cause the layout to be invalid immediately 
     nextLayout->version = _layoutVersion;
   }
@@ -389,7 +394,7 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
 
 - (ASSizeRange)_locked_constrainedSizeForLayoutPass
 {
-  // TODO: The logic in -_setNeedsLayoutFromAbove seems correct and doesn't use this method.
+  // TODO: The logic in -_u_setNeedsLayoutFromAbove seems correct and doesn't use this method.
   // logic seems correct.  For what case does -this method need to do the CGSizeEqual checks?
   // IF WE CAN REMOVE BOUNDS CHECKS HERE, THEN WE CAN ALSO REMOVE "REQUESTED FROM ABOVE" CHECK
 

Source/ASDisplayNode.mm

@@ -911,7 +911,9 @@ - (void)__layout
 
     // This method will confirm that the layout is up to date (and update if needed).
     // Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
-    [self _locked_measureNodeWithBoundsIfNecessary:bounds];
+    __instanceLock__.unlock();
+    [self _u_measureNodeWithBoundsIfNecessary:bounds];
+    __instanceLock__.lock();
 
     [self _locked_layoutPlaceholderIfNecessary];
   }

Source/Private/ASDisplayNode+FrameworkPrivate.h

@@ -225,7 +225,7 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyState(ASHierarchyStat
  * @discussion The size of a root node is determined by each subnode. Calling invalidateSize will let the root node know
  * that the intrinsic size of the receiver node is no longer valid and a resizing of the root node needs to happen.
  */
-- (void)_setNeedsLayoutFromAbove;
+- (void)_u_setNeedsLayoutFromAbove;
 
 /**
  * @abstract Subclass hook for nodes that are acting as root nodes. This method is called if one of the subnodes
@@ -237,7 +237,7 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyState(ASHierarchyStat
  * This method will confirm that the layout is up to date (and update if needed).
  * Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
  */
-- (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds;
+- (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds;
 
 /**
  * Layout all of the subnodes based on the sublayouts