Fixes unsafe use of asyncdisplaykit_node. (#2207)

The previous code used either a `@property` with `assign, nonatomic` semantics, or a `__unsafe_unretained` (non-atomic) ivar pointer to an instance for `asyncdisplaykit_node`.

This commit changes those access so that they have the equivalent of `weak, atomic` semantics.

- `_ASDisplayView.mm`
  - Removes the `_node` ivar that was qualified with `__unsafe_unretained`.
  - Removes `@synthesize asyncdisplaykit_node = _node;`.
  - All direct uses of `_node` were replaced with:
    - Creates a strong reference via `ASDisplayNode *node = _asyncdisplaykit_node;`.
    - `s/_node/node/`.
    - These changes were made even if there's a single use of `_asyncdisplaykit_node` as a consistency / defensive measure so that anyone editing this code in the future does not accidentally use a `weak` reference twice.

- `ASDisplayNode.mm`
  - Getters and setters for `asyncdisplaykit_node` were changed from `nonatomic, assign` semantics to the equivalent of `atomic, weak` semantics.
  - `weak` semantics were implemented by using a `ASWeakProxy` object.
  - `objc_setAssociatedObject` was changed from `OBJC_ASSOCIATION_ASSIGN` to `OBJC_ASSOCIATION_RETAIN` (the `atomic` version of retain).

- `ASDisplayNode+FrameworkPrivate.h`
  - Changed the `@property` declarations for `asyncdisplaykit_node` from `nonatomic, assign` to `atomic, weak`.
    - The actual getters and setters are implemented in `ASDisplayNode.mm`, which were changed accordingly.

AsyncDisplayKit/ASDisplayNode.mm

@@ -32,6 +32,7 @@
 #import "ASLayoutSpec.h"
 #import "ASLayoutValidation.h"
 #import "ASCellNode+Internal.h"
+#import "ASWeakProxy.h"
 
 NSInteger const ASDefaultDrawingPriority = ASDefaultTransactionPriority;
 NSString * const ASRenderingEngineDidDisplayScheduledNodesNotification = @"ASRenderingEngineDidDisplayScheduledNodes";
@@ -3353,12 +3354,14 @@ static const char *ASDisplayNodeAssociatedNodeKey = "ASAssociatedNode";
 
 - (void)setAsyncdisplaykit_node:(ASDisplayNode *)node
 {
-  objc_setAssociatedObject(self, ASDisplayNodeAssociatedNodeKey, node, OBJC_ASSOCIATION_ASSIGN); // Weak reference to avoid cycle, since the node retains the view.
+  ASWeakProxy *weakProxy = [ASWeakProxy weakProxyWithTarget:node];
+  objc_setAssociatedObject(self, ASDisplayNodeAssociatedNodeKey, weakProxy, OBJC_ASSOCIATION_RETAIN); // Weak reference to avoid cycle, since the node retains the view.
 }
 
 - (ASDisplayNode *)asyncdisplaykit_node
 {
-  return objc_getAssociatedObject(self, ASDisplayNodeAssociatedNodeKey);
+  ASWeakProxy *weakProxy = objc_getAssociatedObject(self, ASDisplayNodeAssociatedNodeKey);
+  return weakProxy.target;
 }
 
 @end
@@ -3367,12 +3370,14 @@ static const char *ASDisplayNodeAssociatedNodeKey = "ASAssociatedNode";
 
 - (void)setAsyncdisplaykit_node:(ASDisplayNode *)node
 {
-  objc_setAssociatedObject(self, ASDisplayNodeAssociatedNodeKey, node, OBJC_ASSOCIATION_ASSIGN); // Weak reference to avoid cycle, since the node retains the layer.
+  ASWeakProxy *weakProxy = [ASWeakProxy weakProxyWithTarget:node];
+  objc_setAssociatedObject(self, ASDisplayNodeAssociatedNodeKey, weakProxy, OBJC_ASSOCIATION_RETAIN); // Weak reference to avoid cycle, since the node retains the layer.
 }
 
 - (ASDisplayNode *)asyncdisplaykit_node
 {
-  return objc_getAssociatedObject(self, ASDisplayNodeAssociatedNodeKey);
+  ASWeakProxy *weakProxy = objc_getAssociatedObject(self, ASDisplayNodeAssociatedNodeKey);
+  return weakProxy.target;
 }
 
 @end

AsyncDisplayKit/Details/_ASDisplayView.mm

@@ -18,7 +18,7 @@
 #import "ASObjectDescriptionHelpers.h"
 
 @interface _ASDisplayView ()
-@property (nonatomic, assign, readwrite) ASDisplayNode *asyncdisplaykit_node;
+@property (nullable, atomic, weak, readwrite) ASDisplayNode *asyncdisplaykit_node;
 
 // Keep the node alive while its view is active.  If you create a view, add its layer to a layer hierarchy, then release
 // the view, the layer retains the view to prevent a crash.  This replicates this behaviour for the node abstraction.
@@ -27,8 +27,6 @@
 
 @implementation _ASDisplayView
 {
-  __unsafe_unretained ASDisplayNode *_node;  // Though UIView has a .node property added via category, since we can add an ivar to a subclass, use that for performance.
-
   BOOL _inHitTest;
   BOOL _inPointInside;
 
@@ -36,8 +34,6 @@
   CGRect _lastAccessibleElementsFrame;
 }
 
-@synthesize asyncdisplaykit_node = _node;
-
 + (Class)layerClass
 {
   return [_ASDisplayLayer class];
@@ -50,7 +46,7 @@
 {
   NSMutableString *description = [[super description] mutableCopy];
 
-  ASDisplayNode *node = _node;
+  ASDisplayNode *node = _asyncdisplaykit_node;
 
   if (node != nil) {
     NSString *classString = [NSString stringWithFormat:@"%@-", [node class]];
@@ -77,17 +73,19 @@
 
 - (void)willMoveToWindow:(UIWindow *)newWindow
 {
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   BOOL visible = (newWindow != nil);
-  if (visible && !_node.inHierarchy) {
-    [_node __enterHierarchy];
+  if (visible && !node.inHierarchy) {
+    [node __enterHierarchy];
   }
 }
 
 - (void)didMoveToWindow
 {
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   BOOL visible = (self.window != nil);
-  if (!visible && _node.inHierarchy) {
-    [_node __exitHierarchy];
+  if (!visible && node.inHierarchy) {
+    [node __exitHierarchy];
   }
 }
 
@@ -97,13 +95,14 @@
   // display their contents as long as they are visible somewhere, and aids in lifecycle management because the
   // lifecycle of the node can be treated as the same as the lifecycle of the view (let the view hierarchy own the
   // view).
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   UIView *currentSuperview = self.superview;
   if (!currentSuperview && newSuperview) {
-    self.keepalive_node = _node;
+    self.keepalive_node = node;
   }
   
   if (newSuperview) {
-    ASDisplayNode *supernode = _node.supernode;
+    ASDisplayNode *supernode = node.supernode;
     BOOL supernodeLoaded = supernode.nodeLoaded;
     ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for _ASDisplayView's supernode to be layer-backed.");
     
@@ -128,28 +127,29 @@
 
     if (needsSupernodeUpdate) {
       // -removeFromSupernode is called by -addSubnode:, if it is needed.
-      [newSuperview.asyncdisplaykit_node addSubnode:_node];
+      [newSuperview.asyncdisplaykit_node addSubnode:node];
     }
   }
 }
 
 - (void)didMoveToSuperview
 {
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   UIView *superview = self.superview;
   if (superview == nil) {
     // Clearing keepalive_node may cause deallocation of the node.  In this case, __exitHierarchy may not have an opportunity (e.g. _node will be cleared
     // by the time -didMoveToWindow occurs after this) to clear the Visible interfaceState, which we need to do before deallocation to meet an API guarantee.
-    if (_node.inHierarchy) {
-      [_node __exitHierarchy];
+    if (node.inHierarchy) {
+      [node __exitHierarchy];
     }
     self.keepalive_node = nil;
   }
 
-  ASDisplayNode *supernode = _node.supernode;
+  ASDisplayNode *supernode = node.supernode;
   ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for superview's node to be layer-backed.");
   
   if (supernode) {
-    ASDisplayNodeAssertTrue(_node.nodeLoaded);
+    ASDisplayNodeAssertTrue(node.nodeLoaded);
     BOOL supernodeLoaded = supernode.nodeLoaded;
     BOOL needsSupernodeRemoval = NO;
     
@@ -171,13 +171,13 @@
       // If supernode is loaded but our superview is nil, the user likely manually removed us, so disconnect supernode.
       // The unlikely alternative: we are in __unloadNode, with shouldRasterizeSubnodes just having been turned on.
       // In the latter case, we don't want to disassemble the node hierarchy because all views are intentionally being destroyed.
-      BOOL nodeIsRasterized = ((_node.hierarchyState & ASHierarchyStateRasterized) == ASHierarchyStateRasterized);
+      BOOL nodeIsRasterized = ((node.hierarchyState & ASHierarchyStateRasterized) == ASHierarchyStateRasterized);
       needsSupernodeRemoval = (supernodeLoaded && !nodeIsRasterized);
     }
     
     if (needsSupernodeRemoval) {
       // The node will only disconnect from its supernode, not removeFromSuperview, in this condition.
-      [_node removeFromSupernode];
+      [node removeFromSupernode];
     }
   }
 }
@@ -222,15 +222,17 @@
 
 - (void)setBounds:(CGRect)bounds
 {
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   [super setBounds:bounds];
-  _node.threadSafeBounds = bounds;
+  node.threadSafeBounds = bounds;
 }
 
 #pragma mark - Event Handling + UIResponder Overrides
 - (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event
 {
-  if (_node.methodOverrides & ASDisplayNodeMethodOverrideTouchesBegan) {
-    [_node touchesBegan:touches withEvent:event];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  if (node.methodOverrides & ASDisplayNodeMethodOverrideTouchesBegan) {
+    [node touchesBegan:touches withEvent:event];
   } else {
     [super touchesBegan:touches withEvent:event];
   }
@@ -238,8 +240,9 @@
 
 - (void)touchesMoved:(NSSet *)touches withEvent:(UIEvent *)event
 {
-  if (_node.methodOverrides & ASDisplayNodeMethodOverrideTouchesMoved) {
-    [_node touchesMoved:touches withEvent:event];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  if (node.methodOverrides & ASDisplayNodeMethodOverrideTouchesMoved) {
+    [node touchesMoved:touches withEvent:event];
   } else {
     [super touchesMoved:touches withEvent:event];
   }
@@ -247,8 +250,9 @@
 
 - (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event
 {
-  if (_node.methodOverrides & ASDisplayNodeMethodOverrideTouchesEnded) {
-    [_node touchesEnded:touches withEvent:event];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  if (node.methodOverrides & ASDisplayNodeMethodOverrideTouchesEnded) {
+    [node touchesEnded:touches withEvent:event];
   } else {
     [super touchesEnded:touches withEvent:event];
   }
@@ -256,8 +260,9 @@
 
 - (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event
 {
-  if (_node.methodOverrides & ASDisplayNodeMethodOverrideTouchesCancelled) {
-    [_node touchesCancelled:touches withEvent:event];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  if (node.methodOverrides & ASDisplayNodeMethodOverrideTouchesCancelled) {
+    [node touchesCancelled:touches withEvent:event];
   } else {
     [super touchesCancelled:touches withEvent:event];
   }
@@ -292,9 +297,10 @@
   // Set boolean so this method can be re-entrant.  If the node subclass wants to default to / make use of UIView
   // hitTest:, it will call it on the view, which is _ASDisplayView.  After calling into the node, any additional calls
   // should use the UIView implementation of hitTest:
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   if (!_inHitTest) {
     _inHitTest = YES;
-    UIView *hitView = [_node hitTest:point withEvent:event];
+    UIView *hitView = [node hitTest:point withEvent:event];
     _inHitTest = NO;
     return hitView;
   } else {
@@ -305,9 +311,10 @@
 - (BOOL)pointInside:(CGPoint)point withEvent:(UIEvent *)event
 {
   // See comments in -hitTest:withEvent: for the strategy here.
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
   if (!_inPointInside) {
     _inPointInside = YES;
-    BOOL result = [_node pointInside:point withEvent:event];
+    BOOL result = [node pointInside:point withEvent:event];
     _inPointInside = NO;
     return result;
   } else {
@@ -318,34 +325,40 @@
 #if __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_6_0
 - (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer
 {
-  return [_node gestureRecognizerShouldBegin:gestureRecognizer];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node gestureRecognizerShouldBegin:gestureRecognizer];
 }
 #endif
 
 - (void)asyncdisplaykit_asyncTransactionContainerStateDidChange
 {
-  [_node asyncdisplaykit_asyncTransactionContainerStateDidChange];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  [node asyncdisplaykit_asyncTransactionContainerStateDidChange];
 }
 
 - (void)tintColorDidChange
 {
+    ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
     [super tintColorDidChange];
     
-    [_node tintColorDidChange];
+    [node tintColorDidChange];
 }
 
 - (BOOL)canBecomeFirstResponder {
-    return [_node canBecomeFirstResponder];
+    ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+    return [node canBecomeFirstResponder];
 }
 
 - (BOOL)canResignFirstResponder {
-    return [_node canResignFirstResponder];
+    ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+    return [node canResignFirstResponder];
 }
 
 - (BOOL)canPerformAction:(SEL)action withSender:(id)sender
 {
   // We forward responder-chain actions to our node if we can't handle them ourselves. See -targetForAction:withSender:.
-  return ([super canPerformAction:action withSender:sender] || [_node respondsToSelector:action]);
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return ([super canPerformAction:action withSender:sender] || [node respondsToSelector:action]);
 }
 
 - (id)forwardingTargetForSelector:(SEL)aSelector
@@ -353,39 +366,46 @@
   // Ideally, we would implement -targetForAction:withSender: and simply return the node where we don't respond personally.
   // Unfortunately UIResponder's default implementation of -targetForAction:withSender: doesn't follow its own documentation. It doesn't call -targetForAction:withSender: up the responder chain when -canPerformAction:withSender: fails, but instead merely calls -canPerformAction:withSender: on itself and then up the chain. rdar://20111500.
   // Consequently, to forward responder-chain actions to our node, we override -canPerformAction:withSender: (used by the chain) to indicate support for responder chain-driven actions that our node supports, and then provide the node as a forwarding target here.
-  return _node;
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return node;
 }
 
 #if TARGET_OS_TV
 #pragma mark - tvOS
 - (BOOL)canBecomeFocused
 {
-  return [_node canBecomeFocused];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node canBecomeFocused];
 }
 
 - (void)didUpdateFocusInContext:(UIFocusUpdateContext *)context withAnimationCoordinator:(UIFocusAnimationCoordinator *)coordinator
 {
-  return [_node didUpdateFocusInContext:context withAnimationCoordinator:coordinator];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node didUpdateFocusInContext:context withAnimationCoordinator:coordinator];
 }
 
 - (void)setNeedsFocusUpdate
 {
-  return [_node setNeedsFocusUpdate];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node setNeedsFocusUpdate];
 }
 
 - (void)updateFocusIfNeeded
 {
-  return [_node updateFocusIfNeeded];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node updateFocusIfNeeded];
 }
 
 - (BOOL)shouldUpdateFocusInContext:(UIFocusUpdateContext *)context
 {
-  return [_node shouldUpdateFocusInContext:context];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node shouldUpdateFocusInContext:context];
 }
 
 - (UIView *)preferredFocusedView
 {
-  return [_node preferredFocusedView];
+  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
+  return [node preferredFocusedView];
 }
 #endif
 @end

AsyncDisplayKit/Private/ASDisplayNode+FrameworkPrivate.h

@@ -142,11 +142,11 @@ inline BOOL ASHierarchyStateIncludesRangeManaged(ASHierarchyState hierarchyState
 @end
 
 @interface UIView (ASDisplayNodeInternal)
-@property (nullable, nonatomic, assign, readwrite) ASDisplayNode *asyncdisplaykit_node;
+@property (nullable, atomic, weak, readwrite) ASDisplayNode *asyncdisplaykit_node;
 @end
 
 @interface CALayer (ASDisplayNodeInternal)
-@property (nullable, nonatomic, assign, readwrite) ASDisplayNode *asyncdisplaykit_node;
+@property (nullable, atomic, weak, readwrite) ASDisplayNode *asyncdisplaykit_node;
 @end
 
 NS_ASSUME_NONNULL_END