Have node and controller share lock (TextureGroup#1227)

* Have node and controller share lock * Do it different * Restore prior method order * Maybe store the reference * Try that
rcancro · Nov 15, 2018 · 2baa943 · 2baa943
1 parent 8029632
commit 2baa943
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 20 deletions.
diff --git a/Source/ASNodeController+Beta.h b/Source/ASNodeController+Beta.h
@@ -10,18 +10,21 @@
 #import <AsyncDisplayKit/ASDisplayNode.h>
 #import <AsyncDisplayKit/ASDisplayNode+Subclasses.h> // for ASInterfaceState protocol
 
+NS_ASSUME_NONNULL_BEGIN
+
 /* ASNodeController is currently beta and open to change in the future */
 @interface ASNodeController<__covariant DisplayNodeType : ASDisplayNode *>
-    : NSObject <ASInterfaceStateDelegate, NSLocking>
+    : NSObject <ASInterfaceStateDelegate, ASLocking>
 
-@property (nonatomic, strong /* may be weak! */) DisplayNodeType node;
+@property (strong, readonly /* may be weak! */) DisplayNodeType node;
 
 // Until an ASNodeController can be provided in place of an ASCellNode, some apps may prefer to have
 // nodes keep their controllers alive (and a weak reference from controller to node)
 
 @property (nonatomic) BOOL shouldInvertStrongReference;
 
-- (void)loadNode;
+// called on an arbitrary thread by the framework. You do not call this. Return a new node instance.
+- (DisplayNodeType)createNode;
 
 // for descriptions see <ASInterfaceState> definition
 - (void)nodeDidLoad ASDISPLAYNODE_REQUIRES_SUPER;
@@ -48,6 +51,8 @@
 
 @interface ASDisplayNode (ASNodeController)
 
-@property(nonatomic, readonly) ASNodeController *nodeController;
+@property(nullable, readonly) ASNodeController *nodeController;
 
 @end
+
+NS_ASSUME_NONNULL_END
diff --git a/Source/ASNodeController+Beta.mm b/Source/ASNodeController+Beta.mm
@@ -18,22 +18,27 @@ @implementation ASNodeController
 {
   ASDisplayNode *_strongNode;
   __weak ASDisplayNode *_weakNode;
-  ASDN::RecursiveMutex __instanceLock__;
+  ASDN::Mutex _nodeLock;
 }
 
-- (void)loadNode
+- (ASDisplayNode *)createNode
 {
-  ASLockScopeSelf();
-  self.node = [[ASDisplayNode alloc] init];
+  return [[ASDisplayNode alloc] init];
 }
 
 - (ASDisplayNode *)node
 {
-  ASLockScopeSelf();
-  if (_node == nil) {
-    [self loadNode];
+  ASDN::MutexLocker l(_nodeLock);
+  ASDisplayNode *node = _node;
+  if (!node) {
+    node = [self createNode];
+    if (!node) {
+      ASDisplayNodeCFailAssert(@"Returned nil from -createNode.");
+      node = [[ASDisplayNode alloc] init];
+    }
+    [self setupReferencesWithNode:node];
   }
-  return _node;
+  return node;
 }
 
 - (void)setupReferencesWithNode:(ASDisplayNode *)node
@@ -53,12 +58,6 @@ - (void)setupReferencesWithNode:(ASDisplayNode *)node
   [node addInterfaceStateDelegate:self];
 }
 
-- (void)setNode:(ASDisplayNode *)node
-{
-  ASLockScopeSelf();
-  [self setupReferencesWithNode:node];
-}
-
 - (void)setShouldInvertStrongReference:(BOOL)shouldInvertStrongReference
 {
   ASLockScopeSelf();
@@ -93,12 +92,19 @@ - (void)hierarchyDisplayDidFinish {}
 
 - (void)lock
 {
-  __instanceLock__.lock();
+  [self.node lock];
 }
 
 - (void)unlock
 {
-  __instanceLock__.unlock();
+  // Since the node was already locked on this thread, we don't need to call our accessor or take our lock.
+  ASDisplayNodeAssertNotNil(_node, @"Node deallocated while locked.");
+  [_node unlock];
+}
+
+- (BOOL)tryLock
+{
+  return [self.node tryLock];
 }
 
 @end