Make objects conform to NSLocking (#851)

* Make display node, layout spec, and style conform to NSLocking so that users/subclasses can access their locks * Update the changelog * Align slashes * Put it back, when we're in ASDisplayNode * Go a little further * Put back the changes I didn't mean to commit * Kick the CI * Fix yoga build * Put back non-locking change * Address comments from Scott
TextureGroup · Mar 25, 2018 · 0f9b1e6 · 0f9b1e6
1 parent 255aec5
commit 0f9b1e6
Show file tree

Hide file tree

Showing 29 changed files with 461 additions and 527 deletions.
diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj
@@ -424,7 +424,6 @@
 		DBDB83971C6E879900D0098C /* ASPagerFlowLayout.m in Sources */ = {isa = PBXBuildFile; fileRef = DBDB83931C6E879900D0098C /* ASPagerFlowLayout.m */; };
 		DE4843DC1C93EAC100A1F33B /* ASLayoutTransition.h in Headers */ = {isa = PBXBuildFile; fileRef = E52405B41C8FEF16004DC8E7 /* ASLayoutTransition.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		DE6EA3231C14000600183B10 /* ASDisplayNode+FrameworkPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = DE6EA3211C14000600183B10 /* ASDisplayNode+FrameworkPrivate.h */; settings = {ATTRIBUTES = (Private, ); }; };
-		DE7EF4F81DFF77720082B84A /* ASDisplayNode+FrameworkSubclasses.h in Headers */ = {isa = PBXBuildFile; fileRef = DE7EF4F71DFF77720082B84A /* ASDisplayNode+FrameworkSubclasses.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		DE84918D1C8FFF2B003D89E9 /* ASRunLoopQueue.h in Headers */ = {isa = PBXBuildFile; fileRef = 81EE384D1C8E94F000456208 /* ASRunLoopQueue.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		DE84918E1C8FFF9F003D89E9 /* ASRunLoopQueue.mm in Sources */ = {isa = PBXBuildFile; fileRef = 81EE384E1C8E94F000456208 /* ASRunLoopQueue.mm */; };
 		DE8BEAC21C2DF3FC00D57C12 /* ASDelegateProxy.h in Headers */ = {isa = PBXBuildFile; fileRef = DE8BEABF1C2DF3FC00D57C12 /* ASDelegateProxy.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -933,7 +932,6 @@
 		DBDB83921C6E879900D0098C /* ASPagerFlowLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPagerFlowLayout.h; sourceTree = "<group>"; };
 		DBDB83931C6E879900D0098C /* ASPagerFlowLayout.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPagerFlowLayout.m; sourceTree = "<group>"; };
 		DE6EA3211C14000600183B10 /* ASDisplayNode+FrameworkPrivate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASDisplayNode+FrameworkPrivate.h"; sourceTree = "<group>"; };
-		DE7EF4F71DFF77720082B84A /* ASDisplayNode+FrameworkSubclasses.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASDisplayNode+FrameworkSubclasses.h"; sourceTree = "<group>"; };
 		DE8BEABF1C2DF3FC00D57C12 /* ASDelegateProxy.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASDelegateProxy.h; sourceTree = "<group>"; };
 		DE8BEAC01C2DF3FC00D57C12 /* ASDelegateProxy.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASDelegateProxy.m; sourceTree = "<group>"; };
 		DEC146B41C37A16A004A0EE7 /* ASCollectionInternal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ASCollectionInternal.h; path = Details/ASCollectionInternal.h; sourceTree = "<group>"; };
@@ -1421,7 +1419,6 @@
 				058D0A09195D050800B7D73C /* ASDisplayNode+DebugTiming.h */,
 				058D0A0A195D050800B7D73C /* ASDisplayNode+DebugTiming.mm */,
 				DE6EA3211C14000600183B10 /* ASDisplayNode+FrameworkPrivate.h */,
-				DE7EF4F71DFF77720082B84A /* ASDisplayNode+FrameworkSubclasses.h */,
 				058D0A0B195D050800B7D73C /* ASDisplayNode+UIViewBridge.mm */,
 				058D0A0C195D050800B7D73C /* ASDisplayNodeInternal.h */,
 				6959433D1D70815300B0EE1F /* ASDisplayNodeLayout.h */,
@@ -1849,7 +1846,6 @@
 				34EFC7691B701CE100AD841F /* ASLayoutElement.h in Headers */,
 				698DFF471E36B7E9002891F1 /* ASLayoutSpecUtilities.h in Headers */,
 				9C70F20D1CDBE9CB007D6C76 /* ASDefaultPlayButton.h in Headers */,
-				DE7EF4F81DFF77720082B84A /* ASDisplayNode+FrameworkSubclasses.h in Headers */,
 				CCCCCCD51EC3EF060087FE10 /* ASTextDebugOption.h in Headers */,
 				CC034A091E60BEB400626263 /* ASDisplayNode+Convenience.h in Headers */,
 				254C6B7E1BF94DF4003EC431 /* ASTextKitTailTruncater.h in Headers */,

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,7 @@
 - Fix ASTextNode2 handling background color incorrectly. [Adlai Holler](https://github.com/Adlai-Holler) [#831](https://github.com/TextureGroup/Texture/pull/831/)
 - [NoCopyRendering] Improved performance & fixed image memory not being tagged in Instruments. [Adlai Holler](https://github.com/Adlai-Holler) [#833](https://github.com/TextureGroup/Texture/pull/833/)
 - Use `NS_RETURNS_RETAINED` macro to make our methods a tiny bit faster. [Adlai Holler](https://github.com/Adlai-Holler) [#843](https://github.com/TextureGroup/Texture/pull/843/)
+- `ASDisplayNode, ASLayoutSpec, and ASLayoutElementStyle` now conform to `NSLocking`. They act as recursive locks. Useful locking macros have been added as `ASThread.h`. Subclasses / client code can lock these objects but should be careful as usual when dealing with locks. [Adlai Holler](https://github.com/Adlai-Holler)
 
 ## 2.6
 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)

diff --git a/Source/ASButtonNode.mm b/Source/ASButtonNode.mm
@@ -18,7 +18,7 @@
 #import <AsyncDisplayKit/ASButtonNode.h>
 #import <AsyncDisplayKit/ASStackLayoutSpec.h>
 #import <AsyncDisplayKit/ASThread.h>
-#import <AsyncDisplayKit/ASDisplayNode+FrameworkSubclasses.h>
+#import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
 #import <AsyncDisplayKit/ASBackgroundLayoutSpec.h>
 #import <AsyncDisplayKit/ASInsetLayoutSpec.h>
 #import <AsyncDisplayKit/ASAbsoluteLayoutSpec.h>
@@ -161,7 +161,7 @@ - (void)setDisplaysAsynchronously:(BOOL)displaysAsynchronously
 
 - (void)updateImage
 {
-  __instanceLock__.lock();
+  [self lock];
 
   UIImage *newImage;
   if (self.enabled == NO && _disabledImage) {
@@ -178,18 +178,18 @@ - (void)updateImage
 
   if ((_imageNode != nil || newImage != nil) && newImage != self.imageNode.image) {
     _imageNode.image = newImage;
-    __instanceLock__.unlock();
+    [self unlock];
 
     [self setNeedsLayout];
     return;
   }
 
-  __instanceLock__.unlock();
+  [self unlock];
 }
 
 - (void)updateTitle
 {
-  __instanceLock__.lock();
+  [self lock];
 
   NSAttributedString *newTitle;
   if (self.enabled == NO && _disabledAttributedTitle) {
@@ -207,19 +207,19 @@ - (void)updateTitle
   // Calling self.titleNode is essential here because _titleNode is lazily created by the getter.
   if ((_titleNode != nil || newTitle.length > 0) && [self.titleNode.attributedText isEqualToAttributedString:newTitle] == NO) {
     _titleNode.attributedText = newTitle;
-    __instanceLock__.unlock();
+    [self unlock];
 
     self.accessibilityLabel = _titleNode.accessibilityLabel;
     [self setNeedsLayout];
     return;
   }
 
-  __instanceLock__.unlock();
+  [self unlock];
 }
 
 - (void)updateBackgroundImage
 {
-  __instanceLock__.lock();
+  [self lock];
 
   UIImage *newImage;
   if (self.enabled == NO && _disabledBackgroundImage) {
@@ -236,25 +236,25 @@ - (void)updateBackgroundImage
 
   if ((_backgroundImageNode != nil || newImage != nil) && newImage != self.backgroundImageNode.image) {
     _backgroundImageNode.image = newImage;
-    __instanceLock__.unlock();
+    [self unlock];
 
     [self setNeedsLayout];
     return;
   }
 
-  __instanceLock__.unlock();
+  [self unlock];
 }
 
 - (CGFloat)contentSpacing
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _contentSpacing;
 }
 
 - (void)setContentSpacing:(CGFloat)contentSpacing
 {
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     if (contentSpacing == _contentSpacing) {
       return;
     }
@@ -267,14 +267,14 @@ - (void)setContentSpacing:(CGFloat)contentSpacing
 
 - (BOOL)laysOutHorizontally
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _laysOutHorizontally;
 }
 
 - (void)setLaysOutHorizontally:(BOOL)laysOutHorizontally
 {
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     if (laysOutHorizontally == _laysOutHorizontally) {
       return;
     }
@@ -287,49 +287,49 @@ - (void)setLaysOutHorizontally:(BOOL)laysOutHorizontally
 
 - (ASVerticalAlignment)contentVerticalAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _contentVerticalAlignment;
 }
 
 - (void)setContentVerticalAlignment:(ASVerticalAlignment)contentVerticalAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   _contentVerticalAlignment = contentVerticalAlignment;
 }
 
 - (ASHorizontalAlignment)contentHorizontalAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _contentHorizontalAlignment;
 }
 
 - (void)setContentHorizontalAlignment:(ASHorizontalAlignment)contentHorizontalAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   _contentHorizontalAlignment = contentHorizontalAlignment;
 }
 
 - (UIEdgeInsets)contentEdgeInsets
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _contentEdgeInsets;
 }
 
 - (void)setContentEdgeInsets:(UIEdgeInsets)contentEdgeInsets
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   _contentEdgeInsets = contentEdgeInsets;
 }
 
 - (ASButtonNodeImageAlignment)imageAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   return _imageAlignment;
 }
 
 - (void)setImageAlignment:(ASButtonNodeImageAlignment)imageAlignment
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   _imageAlignment = imageAlignment;
 }
 
@@ -349,7 +349,7 @@ - (void)setTitle:(NSString *)title withFont:(UIFont *)font withColor:(UIColor *)
 
 - (NSAttributedString *)attributedTitleForState:(UIControlState)state
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   switch (state) {
     case UIControlStateNormal:
       return _normalAttributedTitle;
@@ -374,7 +374,7 @@ - (NSAttributedString *)attributedTitleForState:(UIControlState)state
 - (void)setAttributedTitle:(NSAttributedString *)title forState:(UIControlState)state
 {
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     switch (state) {
       case UIControlStateNormal:
         _normalAttributedTitle = [title copy];
@@ -406,7 +406,7 @@ - (void)setAttributedTitle:(NSAttributedString *)title forState:(UIControlState)
 
 - (UIImage *)imageForState:(UIControlState)state
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   switch (state) {
     case UIControlStateNormal:
       return _normalImage;
@@ -431,7 +431,7 @@ - (UIImage *)imageForState:(UIControlState)state
 - (void)setImage:(UIImage *)image forState:(UIControlState)state
 {
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     switch (state) {
       case UIControlStateNormal:
         _normalImage = image;
@@ -463,7 +463,7 @@ - (void)setImage:(UIImage *)image forState:(UIControlState)state
 
 - (UIImage *)backgroundImageForState:(UIControlState)state
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASLockScopeSelf();
   switch (state) {
     case UIControlStateNormal:
       return _normalBackgroundImage;
@@ -488,7 +488,7 @@ - (UIImage *)backgroundImageForState:(UIControlState)state
 - (void)setBackgroundImage:(UIImage *)image forState:(UIControlState)state
 {
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     switch (state) {
       case UIControlStateNormal:
         _normalBackgroundImage = image;
@@ -525,7 +525,7 @@ - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize
   ASLayoutSpec *spec;
   ASStackLayoutSpec *stack = [[ASStackLayoutSpec alloc] init];
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASLockScopeSelf();
     stack.direction = _laysOutHorizontally ? ASStackLayoutDirectionHorizontal : ASStackLayoutDirectionVertical;
     stack.spacing = _contentSpacing;
     stack.horizontalAlignment = _contentHorizontalAlignment;

diff --git a/Source/ASControlNode.mm b/Source/ASControlNode.mm
@@ -38,8 +38,6 @@
 @interface ASControlNode ()
 {
 @private
-  ASDN::RecursiveMutex _controlLock;
-
   // Control Attributes
   BOOL _enabled;
   BOOL _highlighted;
@@ -298,7 +296,7 @@ - (void)addTarget:(id)target action:(SEL)action forControlEvents:(ASControlNodeE
   // ASControlNode cannot be layer backed if adding a target
   ASDisplayNodeAssert(!self.isLayerBacked, @"ASControlNode is layer backed, will never be able to call target in target:action: pair.");
 
-  ASDN::MutexLocker l(_controlLock);
+  ASLockScopeSelf();
 
   if (!_controlEventDispatchTable) {
     _controlEventDispatchTable = [[NSMutableDictionary alloc] initWithCapacity:kASControlNodeEventDispatchTableInitialCapacity]; // enough to handle common types without re-hashing the dictionary when adding entries.
@@ -352,7 +350,7 @@ - (NSArray *)actionsForTarget:(id)target forControlEvent:(ASControlNodeEvent)con
   NSParameterAssert(target);
   NSParameterAssert(controlEvent != 0 && controlEvent != ASControlNodeEventAllEvents);
 
-  ASDN::MutexLocker l(_controlLock);
+  ASLockScopeSelf();
 
   // Grab the event target action array for this event.
   NSMutableArray *eventTargetActionArray = _controlEventDispatchTable[_ASControlNodeEventKeyForControlEvent(controlEvent)];
@@ -374,7 +372,7 @@ - (NSArray *)actionsForTarget:(id)target forControlEvent:(ASControlNodeEvent)con
 
 - (NSSet *)allTargets
 {
-  ASDN::MutexLocker l(_controlLock);
+  ASLockScopeSelf();
 
   NSMutableSet *targets = [[NSMutableSet alloc] init];
 
@@ -393,7 +391,7 @@ - (void)removeTarget:(id)target action:(SEL)action forControlEvents:(ASControlNo
 {
   NSParameterAssert(controlEventMask != 0);
 
-  ASDN::MutexLocker l(_controlLock);
+  ASLockScopeSelf();
 
   // Enumerate the events in the mask, removing the target-action pair for each control event included in controlEventMask.
   _ASEnumerateControlEventsIncludedInMaskWithBlock(controlEventMask, ^
@@ -435,31 +433,31 @@ - (void)sendActionsForControlEvents:(ASControlNodeEvent)controlEvents withEvent:
 
   NSMutableArray *resolvedEventTargetActionArray = [[NSMutableArray<ASControlTargetAction *> alloc] init];
 
-  _controlLock.lock();
-
-  // Enumerate the events in the mask, invoking the target-action pairs for each.
-  _ASEnumerateControlEventsIncludedInMaskWithBlock(controlEvents, ^
-    (ASControlNodeEvent controlEvent)
-    {
-      // Iterate on each target action pair
-      for (ASControlTargetAction *targetAction in _controlEventDispatchTable[_ASControlNodeEventKeyForControlEvent(controlEvent)]) {
-        ASControlTargetAction *resolvedTargetAction = [[ASControlTargetAction alloc] init];
-        resolvedTargetAction.action = targetAction.action;
-        resolvedTargetAction.target = targetAction.target;
-
-        // NSNull means that a nil target was set, so start at self and travel the responder chain
-        if (!resolvedTargetAction.target && targetAction.createdWithNoTarget) {
-          // if the target cannot perform the action, travel the responder chain to try to find something that does
-          resolvedTargetAction.target = [self.view targetForAction:resolvedTargetAction.action withSender:self];
-        }
-
-        if (resolvedTargetAction.target) {
-          [resolvedEventTargetActionArray addObject:resolvedTargetAction];
+  {
+    ASLockScopeSelf();
+
+    // Enumerate the events in the mask, invoking the target-action pairs for each.
+    _ASEnumerateControlEventsIncludedInMaskWithBlock(controlEvents, ^
+      (ASControlNodeEvent controlEvent)
+      {
+        // Iterate on each target action pair
+        for (ASControlTargetAction *targetAction in _controlEventDispatchTable[_ASControlNodeEventKeyForControlEvent(controlEvent)]) {
+          ASControlTargetAction *resolvedTargetAction = [[ASControlTargetAction alloc] init];
+          resolvedTargetAction.action = targetAction.action;
+          resolvedTargetAction.target = targetAction.target;
+
+          // NSNull means that a nil target was set, so start at self and travel the responder chain
+          if (!resolvedTargetAction.target && targetAction.createdWithNoTarget) {
+            // if the target cannot perform the action, travel the responder chain to try to find something that does
+            resolvedTargetAction.target = [self.view targetForAction:resolvedTargetAction.action withSender:self];
+          }
+
+          if (resolvedTargetAction.target) {
+            [resolvedEventTargetActionArray addObject:resolvedTargetAction];
+          }
         }
-      }
-    });
-
-  _controlLock.unlock();
+      });
+  }
 
   //We don't want to hold the lock while calling out, we could potentially walk up the ownership tree causing a deadlock.
 #pragma clang diagnostic push

diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm
@@ -17,14 +17,13 @@
 
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 #import <AsyncDisplayKit/ASDisplayNodeInternal.h>
+#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
+#import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
 #import <AsyncDisplayKit/ASLayout.h>
 #import <AsyncDisplayKit/ASLayoutElementStylePrivate.h>
 #import <AsyncDisplayKit/ASLog.h>
 
-#import <AsyncDisplayKit/ASDisplayNode+FrameworkSubclasses.h>
-
-#pragma mark -
 #pragma mark - ASDisplayNode (ASLayoutElement)
 
 @implementation ASDisplayNode (ASLayoutElement)