Remove direct ivar access on non-self object to fix mocking case #tri…

…vial (#1066) * Remove direct ivar access on non-self object to prevent issues when the object is actually a mock * Comment * Remove lock to avoid deadlock risk
TextureGroup · Aug 3, 2018 · c5b1d09 · c5b1d09
1 parent 40e3bf8
commit c5b1d09
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 6 deletions.
diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -47,6 +47,7 @@
 #import <AsyncDisplayKit/ASLayoutSpecPrivate.h>
 #import <AsyncDisplayKit/ASLog.h>
 #import <AsyncDisplayKit/ASMainThreadDeallocation.h>
+#import <AsyncDisplayKit/ASNodeController+Beta.h>
 #import <AsyncDisplayKit/ASRunLoopQueue.h>
 #import <AsyncDisplayKit/ASSignpost.h>
 #import <AsyncDisplayKit/ASTraitCollection.h>
@@ -876,6 +877,18 @@ - (void)setAutomaticallyRelayoutOnLayoutMarginsChanges:(BOOL)flag
   _automaticallyRelayoutOnLayoutMarginsChanges = flag;
 }
 
+- (void)__setNodeController:(ASNodeController *)controller
+{
+  // See docs for why we don't lock.
+  if (controller.shouldInvertStrongReference) {
+    _strongNodeController = controller;
+    _weakNodeController = nil;
+  } else {
+    _weakNodeController = controller;
+    _strongNodeController = nil;
+  }
+}
+
 #pragma mark - UIResponder
 
 #define HANDLE_NODE_RESPONDER_METHOD(__sel) \

diff --git a/Source/ASNodeController+Beta.mm b/Source/ASNodeController+Beta.mm
@@ -46,17 +46,14 @@ - (void)setupReferencesWithNode:(ASDisplayNode *)node
   if (_shouldInvertStrongReference) {
     // The node should own the controller; weak reference from controller to node.
     _weakNode = node;
-    node->_strongNodeController = self;
-    node->_weakNodeController = nil;
     _strongNode = nil;
   } else {
     // The controller should own the node; weak reference from node to controller.
     _strongNode = node;
-    node->_weakNodeController = self;
-    node->_strongNodeController = nil;
     _weakNode = nil;
   }
 
+  [node __setNodeController:self];
   [node addInterfaceStateDelegate:self];
 }
 

diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h
@@ -93,8 +93,6 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest
   ASInterfaceState _pendingInterfaceState;
   UIView *_view;
   CALayer *_layer;
-  ASNodeController *_strongNodeController;
-  __weak ASNodeController *_weakNodeController;
 
   std::atomic<ASDisplayNodeAtomicFlags> _atomicFlags;
 
@@ -138,6 +136,9 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest
   ASDisplayNode * __weak _supernode;
   NSMutableArray<ASDisplayNode *> *_subnodes;
 
+  ASNodeController *_strongNodeController;
+  __weak ASNodeController *_weakNodeController;
+
   // Set this to nil whenever you modify _subnodes
   NSArray<ASDisplayNode *> *_cachedSubnodes;
 
@@ -276,6 +277,16 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest
  */
 - (void)__setNeedsDisplay;
 
+/**
+ * Setup the node -> controller reference. Strong or weak is based on
+ * the "shouldInvertStrongReference" property of the controller.
+ *
+ * Note: To prevent lock-ordering deadlocks, this method does not take the node's lock.
+ * In practice, changing the node controller of a node multiple times is not
+ * supported behavior.
+ */
+- (void)__setNodeController:(ASNodeController *)controller;
+
 /**
  * Called whenever the node needs to layout its subnodes and, if it's already loaded, its subviews. Executes the layout pass for the node
  *