Improve locking situation in ASVideoPlayerNode (#1042)

* Improve locking in ASVideoPlayerNode

* Address comments

CHANGELOG.md

@@ -23,7 +23,9 @@
 - Split MapKit, Photos, and AssetsLibrary dependent code into separate subspecs to improve binary size and start time when they're not needed. The default subspec includes all three for backwards compatibility, but this **will change in 3.0**. When using non-Cocoapods build environments, define `AS_USE_PHOTOS, AS_USE_MAPKIT, AS_USE_ASSETS_LIBRARY` to 1 respectively to signal their use. [Adlai Holler](https://github.com/Adlai-Holler)
 - Optimization: Removed an NSMutableArray in flattened layouts. [Adlai Holler](https://github.com/Adlai-Holler)
 - Reduced binary size by disabling exception support (which we don't use.) [Adlai Holler](https://github.com/Adlai-Holler)
-- Create and set delegate for clip corner layers within ASDisplayNode [Michael Schneider](https://github.com/maicki) [#1024](https://github.com/TextureGroup/Texture/pull/1029)
+- Create and set delegate for clip corner layers within ASDisplayNode [Michael Schneider](https://github.com/maicki) [#1029](https://github.com/TextureGroup/Texture/pull/1029)
+- Improve locking situation in ASVideoPlayerNode [Michael Schneider](https://github.com/maicki) [#1042](https://github.com/TextureGroup/Texture/pull/1042)
+
 
 ## 2.7
 - Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)

Source/ASVideoPlayerNode.mm

@@ -209,10 +209,8 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 - (void)didLoad
 {
   [super didLoad];
-  {
-    ASLockScopeSelf();
-    [self createControls];
-  }
+  
+  [self createControls];
 }
 
 - (void)didEnterPreloadState
@@ -284,15 +282,23 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
     if (_delegateFlags.delegateCustomControls && _delegateFlags.delegateLayoutSpecForControls) {
       NSDictionary *customControls = [_delegate videoPlayerNodeCustomControls:self];
+      std::vector<ASDisplayNode *> subnodes;
       for (id key in customControls) {
         id node = customControls[key];
         if (![node isKindOfClass:[ASDisplayNode class]]) {
           continue;
         }
 
-        [self addSubnode:node];
+        subnodes.push_back(node);
         [_cachedControls setObject:node forKey:key];
       }
+      
+      {
+        ASUnlockScope(self);
+        for (var subnode : subnodes) {
+          [self addSubnode:subnode];
+        }
+      }
     }
   }
 
@@ -315,14 +321,21 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
 - (void)removeControls
 {
-  for (ASDisplayNode *node in [_cachedControls objectEnumerator]) {
-    [node removeFromSupernode];
+  NSMutableDictionary *cachedControls = nil;
+  {
+    ASLockScope(self);
+  
+    // Grab the cached controls for removing it
+    cachedControls = [_cachedControls copy];
+    [self _locked_cleanCachedControls];
   }
 
-  [self cleanCachedControls];
+  for (ASDisplayNode *node in [cachedControls objectEnumerator]) {
+    [node removeFromSupernode];
+  }
 }
 
-- (void)cleanCachedControls
+- (void)_locked_cleanCachedControls
 {
   [_cachedControls removeAllObjects];
 
@@ -355,7 +368,10 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
     [_cachedControls setObject:_playbackButtonNode forKey:@(ASVideoPlayerNodeControlTypePlaybackButton)];
   }
 
-  [self addSubnode:_playbackButtonNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_playbackButtonNode];
+  }
 }
 
 - (void)_locked_createFullScreenButton
@@ -374,7 +390,10 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
     [_cachedControls setObject:_fullScreenButtonNode forKey:@(ASVideoPlayerNodeControlTypeFullScreenButton)];
   }
   
-  [self addSubnode:_fullScreenButtonNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_fullScreenButtonNode];
+  }
 }
 
 - (void)_locked_createElapsedTextField
@@ -389,7 +408,10 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
     [_cachedControls setObject:_elapsedTextNode forKey:@(ASVideoPlayerNodeControlTypeElapsedText)];
   }
-  [self addSubnode:_elapsedTextNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_elapsedTextNode];
+  }
 }
 
 - (void)_locked_createDurationTextField
@@ -405,7 +427,10 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
     [_cachedControls setObject:_durationTextNode forKey:@(ASVideoPlayerNodeControlTypeDurationText)];
   }
   [self updateDurationTimeLabel];
-  [self addSubnode:_durationTextNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_durationTextNode];
+  }
 }
 
 - (void)_locked_createScrubber
@@ -450,8 +475,10 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
     [_cachedControls setObject:_scrubberNode forKey:@(ASVideoPlayerNodeControlTypeScrubber)];
   }
-
-  [self addSubnode:_scrubberNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_scrubberNode];
+  }
 }
 
 - (void)_locked_createControlFlexGrowSpacer
@@ -623,7 +650,6 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
   ASLockScopeSelf();
 
   if (!_spinnerNode) {
-  
     __weak __typeof__(self) weakSelf = self;
     _spinnerNode = [[ASDisplayNode alloc] initWithViewBlock:^UIView *{
       __typeof__(self) strongSelf = weakSelf;
@@ -642,24 +668,32 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
       
       return spinnnerView;
     }];
-    
     _spinnerNode.style.preferredSize = CGSizeMake(44.0, 44.0);
-
-    [self addSubnode:_spinnerNode];
-    [self setNeedsLayout];
+    
+    let spinnerNode = _spinnerNode;
+    {
+      ASUnlockScope(self);
+      [self addSubnode:spinnerNode];
+      [self setNeedsLayout];
+    }
   }
   [(UIActivityIndicatorView *)_spinnerNode.view startAnimating];
 }
 
 - (void)removeSpinner
 {
-  ASLockScopeSelf();
-
-  if (!_spinnerNode) {
-    return;
+  ASDisplayNode *spinnerNode = nil;
+  {
+    ASLockScopeSelf();
+    if (!_spinnerNode) {
+      return;
+    }
+    
+    spinnerNode = _spinnerNode;
+    _spinnerNode = nil;
   }
-  [_spinnerNode removeFromSupernode];
-  _spinnerNode = nil;
+
+  [spinnerNode removeFromSupernode];
 }
 
 - (void)didTapPlaybackButton:(ASControlNode*)node