ASThread: Remove Locker, Unlocker, and SharedMutex (TextureGroup#1213)

* ASThread: Remove Locker, Unlocker, and SharedMutex * Remove extra line * Kick the CI * Move C++ down * Fix thing
nguyenhuy · Nov 5, 2018 · 5c9815f · 5c9815f
1 parent 5909e27
commit 5c9815f
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 206 deletions.
diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -416,12 +416,13 @@ - (ASDisplayNodeMethodOverrides)methodOverrides
 
 - (void)onDidLoad:(ASDisplayNodeDidLoadBlock)body
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASDN::UniqueLock l(__instanceLock__);
 
   if ([self _locked_isNodeLoaded]) {
     ASDisplayNodeAssertThreadAffinity(self);
-    ASDN::MutexUnlocker l(__instanceLock__);
+    l.unlock();
     body(self);
+    return;
   } else if (_onDidLoadBlocks == nil) {
     _onDidLoadBlocks = [NSMutableArray arrayWithObject:body];
   } else {
@@ -601,7 +602,7 @@ - (BOOL)_locked_isNodeLoaded
 
 - (UIView *)view
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASDN::UniqueLock l(__instanceLock__);
 
   ASDisplayNodeAssert(!_flags.layerBacked, @"Call to -view undefined on layer-backed nodes");
   BOOL isLayerBacked = _flags.layerBacked;
@@ -626,30 +627,28 @@ - (UIView *)view
   // in the background on a loaded node, which isn't currently supported.
   if (_pendingViewState.hasSetNeedsLayout) {
     // Need to unlock before calling setNeedsLayout to avoid deadlocks.
-    // MutexUnlocker will re-lock at the end of scope.
-    ASDN::MutexUnlocker u(__instanceLock__);
+    l.unlock();
     [self __setNeedsLayout];
+    l.lock();
   }
 
   [self _locked_applyPendingStateToViewOrLayer];
 
-  {
-    // The following methods should not be called with a lock
-    ASDN::MutexUnlocker u(__instanceLock__);
+  // The following methods should not be called with a lock
+  l.unlock();
 
-    // No need for the lock as accessing the subviews or layers are always happening on main
-    [self _addSubnodeViewsAndLayers];
-
-    // A subclass hook should never be called with a lock
-    [self _didLoad];
-  }
+  // No need for the lock as accessing the subviews or layers are always happening on main
+  [self _addSubnodeViewsAndLayers];
+
+  // A subclass hook should never be called with a lock
+  [self _didLoad];
 
   return _view;
 }
 
 - (CALayer *)layer
 {
-  ASDN::MutexLocker l(__instanceLock__);
+  ASDN::UniqueLock l(__instanceLock__);
   if (_layer != nil) {
     return _layer;
   }
@@ -661,31 +660,30 @@ - (CALayer *)layer
   // Loading a layer needs to happen on the main thread
   ASDisplayNodeAssertMainThread();
   [self _locked_loadViewOrLayer];
+  CALayer *layer = _layer;
 
   // FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
   // but automatic subnode management would require us to modify the node tree
   // in the background on a loaded node, which isn't currently supported.
   if (_pendingViewState.hasSetNeedsLayout) {
     // Need to unlock before calling setNeedsLayout to avoid deadlocks.
-    // MutexUnlocker will re-lock at the end of scope.
-    ASDN::MutexUnlocker u(__instanceLock__);
+    l.unlock();
     [self __setNeedsLayout];
+    l.lock();
   }
 
   [self _locked_applyPendingStateToViewOrLayer];
 
-  {
-    // The following methods should not be called with a lock
-    ASDN::MutexUnlocker u(__instanceLock__);
+  // The following methods should not be called with a lock
+  l.unlock();
 
-    // No need for the lock as accessing the subviews or layers are always happening on main
-    [self _addSubnodeViewsAndLayers];
-
-    // A subclass hook should never be called with a lock
-    [self _didLoad];
-  }
+  // No need for the lock as accessing the subviews or layers are always happening on main
+  [self _addSubnodeViewsAndLayers];
+
+  // A subclass hook should never be called with a lock
+  [self _didLoad];
 
-  return _layer;
+  return layer;
 }
 
 // Returns nil if the layer is not an _ASDisplayLayer; will not create the layer if nil.
@@ -1033,7 +1031,7 @@ - (void)__layout
 
   BOOL loaded = NO;
   {
-    ASDN::MutexLocker l(__instanceLock__);
+    ASDN::UniqueLock l(__instanceLock__);
     loaded = [self _locked_isNodeLoaded];
     CGRect bounds = _threadSafeBounds;
 
@@ -1055,10 +1053,9 @@ - (void)__layout
 
     // This method will confirm that the layout is up to date (and update if needed).
     // Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
-    {
-      ASDN::MutexUnlocker u(__instanceLock__);
-      [self _u_measureNodeWithBoundsIfNecessary:bounds];
-    }
+    l.unlock();
+    [self _u_measureNodeWithBoundsIfNecessary:bounds];
+    l.lock();
 
     [self _locked_layoutPlaceholderIfNecessary];
   }
@@ -1121,7 +1118,7 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
 {
   __ASDisplayNodeCheckForLayoutMethodOverrides;
 
-  ASDN::MutexLocker l(__instanceLock__);
+  ASDN::UniqueLock l(__instanceLock__);
 
 #if YOGA
   // There are several cases where Yoga could arrive here:
@@ -1138,11 +1135,13 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
     if ([self shouldHaveYogaMeasureFunc] == NO) {
       // If we're a yoga root, tree node, or leaf with no measure func (e.g. spacer), then
       // initiate a new Yoga calculation pass from root.
-      ASDN::MutexUnlocker ul(__instanceLock__);
+
       as_activity_create_for_scope("Yoga layout calculation");
       if (self.yogaLayoutInProgress == NO) {
         ASYogaLog("Calculating yoga layout from root %@, %@", self, NSStringFromASSizeRange(constrainedSize));
+        l.unlock();
         [self calculateLayoutFromYogaRoot:constrainedSize];
+        l.lock();
       } else {
         ASYogaLog("Reusing existing yoga layout %@", _yogaCalculatedLayout);
       }
@@ -3546,15 +3545,15 @@ - (void)applyPendingViewState
   ASDisplayNodeAssertMainThread();
   ASAssertUnlocked(__instanceLock__);
 
-  ASDN::MutexLocker l(__instanceLock__);
+  ASDN::UniqueLock l(__instanceLock__);
   // FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
   // but automatic subnode management would require us to modify the node tree
   // in the background on a loaded node, which isn't currently supported.
   if (_pendingViewState.hasSetNeedsLayout) {
     // Need to unlock before calling setNeedsLayout to avoid deadlocks.
-    // MutexUnlocker will re-lock at the end of scope.
-    ASDN::MutexUnlocker u(__instanceLock__);
+    l.unlock();
     [self __setNeedsLayout];
+    l.lock();
   }
 
   [self _locked_applyPendingViewState];

diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm
@@ -434,12 +434,12 @@ + (UIImage *)displayWithParameters:(id<NSObject>)parameter isCancelled:(NS_NOESC
 
 static ASWeakMap<ASImageNodeContentsKey *, UIImage *> *cache = nil;
 // Allocate cacheLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
-static ASDN::StaticMutex& cacheLock = *new ASDN::StaticMutex;
+static auto *cacheLock = new ASDN::Mutex;
 
 + (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:(id)drawParameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled
 {
   {
-    ASDN::StaticMutexLocker l(cacheLock);
+    ASDN::MutexLocker l(*cacheLock);
     if (!cache) {
       cache = [[ASWeakMap alloc] init];
     }
@@ -456,7 +456,7 @@ + (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:
   }
 
   {
-    ASDN::StaticMutexLocker l(cacheLock);
+    ASDN::MutexLocker l(*cacheLock);
     return [cache setObject:contents forKey:key];
   }
 }

diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm
@@ -55,14 +55,14 @@ @implementation ASTextCacheValue
  */
 static NS_RETURNS_RETAINED ASTextLayout *ASTextNodeCompatibleLayoutWithContainerAndText(ASTextContainer *container, NSAttributedString *text)  {
   // Allocate layoutCacheLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
-  static ASDN::StaticMutex& layoutCacheLock = *new ASDN::StaticMutex;
+  static auto *layoutCacheLock = new ASDN::Mutex;
   static NSCache<NSAttributedString *, ASTextCacheValue *> *textLayoutCache;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     textLayoutCache = [[NSCache alloc] init];
   });
 
-  layoutCacheLock.lock();
+  layoutCacheLock->lock();
 
   ASTextCacheValue *cacheValue = [textLayoutCache objectForKey:text];
   if (cacheValue == nil) {
@@ -72,7 +72,7 @@ @implementation ASTextCacheValue
 
   // Lock the cache item for the rest of the method. Only after acquiring can we release the NSCache.
   ASDN::MutexLocker lock(cacheValue->_m);
-  layoutCacheLock.unlock();
+  layoutCacheLock->unlock();
 
   CGRect containerBounds = (CGRect){ .size = container.size };
   {

diff --git a/Source/ASVideoNode.mm b/Source/ASVideoNode.mm
@@ -10,6 +10,7 @@
 #import <AVFoundation/AVFoundation.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
 #import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
+#import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 #import <AsyncDisplayKit/ASVideoNode.h>
 #import <AsyncDisplayKit/ASEqualityHelpers.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
@@ -322,16 +323,17 @@ - (void)setVideoPlaceholderImage:(UIImage *)image
 
 - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
 {
-  ASLockScopeSelf();
+  ASDN::UniqueLock l(__instanceLock__);
 
   if (object == _currentPlayerItem) {
     if ([keyPath isEqualToString:kStatus]) {
       if ([change[NSKeyValueChangeNewKey] integerValue] == AVPlayerItemStatusReadyToPlay) {
         if (self.playerState != ASVideoNodePlayerStatePlaying) {
           self.playerState = ASVideoNodePlayerStateReadyToPlay;
           if (_shouldBePlaying && ASInterfaceStateIncludesVisible(self.interfaceState)) {
-            ASUnlockScope(self);
+            l.unlock();
             [self play];
+            l.lock();
           }
         }
         // If we don't yet have a placeholder image update it now that we should have data available for it
@@ -354,8 +356,10 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
           [self.delegate videoNodeDidRecoverFromStall:self];
         }
 
-        ASUnlockScope(self);
+        l.unlock();
         [self play]; // autoresume after buffer catches up
+        // NOTE: Early return without re-locking.
+        return;
       }
     } else if ([keyPath isEqualToString:kplaybackBufferEmpty]) {
       if (_shouldBePlaying && [change[NSKeyValueChangeNewKey] boolValue] == YES && ASInterfaceStateIncludesVisible(self.interfaceState)) {
@@ -373,6 +377,8 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
       }
     }
   }
+
+  // NOTE: Early return above.
 }
 
 - (void)tapped

diff --git a/Source/Details/ASBasicImageDownloader.mm b/Source/Details/ASBasicImageDownloader.mm
@@ -39,11 +39,11 @@ @implementation ASBasicImageDownloaderContext
 
 static NSMutableDictionary *currentRequests = nil;
 // Allocate currentRequestsLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
-static ASDN::StaticMutex& currentRequestsLock = *new ASDN::StaticMutex;
+static auto *currentRequestsLock = new ASDN::Mutex;
 
 + (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL
 {
-  ASDN::StaticMutexLocker l(currentRequestsLock);
+  ASDN::MutexLocker l(*currentRequestsLock);
   if (!currentRequests) {
     currentRequests = [[NSMutableDictionary alloc] init];
   }
@@ -57,7 +57,7 @@ + (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL
 
 + (void)cancelContextWithURL:(NSURL *)URL
 {
-  ASDN::StaticMutexLocker l(currentRequestsLock);
+  ASDN::MutexLocker l(*currentRequestsLock);
   if (currentRequests) {
     [currentRequests removeObjectForKey:URL];
   }

diff --git a/Source/Details/ASMainSerialQueue.mm b/Source/Details/ASMainSerialQueue.mm
@@ -40,19 +40,21 @@ - (NSUInteger)numberOfScheduledBlocks
 
 - (void)performBlockOnMainThread:(dispatch_block_t)block
 {
-  ASDN::MutexLocker l(_serialQueueLock);
+
+  ASDN::UniqueLock l(_serialQueueLock);
   [_blocks addObject:block];
   {
-    ASDN::MutexUnlocker u(_serialQueueLock);
+    l.unlock();
     [self runBlocks];
+    l.lock();
   }
 }
 
 - (void)runBlocks
 {
   dispatch_block_t mainThread = ^{
+    ASDN::UniqueLock l(self->_serialQueueLock);
     do {
-      ASDN::MutexLocker l(self->_serialQueueLock);
       dispatch_block_t block;
       if (self->_blocks.count > 0) {
         block = _blocks[0];
@@ -61,8 +63,9 @@ - (void)runBlocks
         break;
       }
       {
-        ASDN::MutexUnlocker u(self->_serialQueueLock);
+        l.unlock();
         block();
+        l.lock();
       }
     } while (true);
   };