From a97714bb4c5933489999db82ecc7bd534c61d829 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Nov 2018 09:46:21 -0800 Subject: [PATCH 1/5] ASThread: Remove Locker, Unlocker, and SharedMutex --- Source/ASDisplayNode.mm | 66 ++++++----- Source/ASImageNode.mm | 6 +- Source/ASTextNode2.mm | 6 +- Source/ASVideoNode.mm | 11 +- Source/Details/ASBasicImageDownloader.mm | 6 +- Source/Details/ASMainSerialQueue.mm | 11 +- Source/Details/ASThread.h | 138 +---------------------- Source/Private/ASLayoutTransition.mm | 18 +-- Source/TextKit/ASTextKitContext.mm | 8 +- 9 files changed, 73 insertions(+), 197 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 34ff77b94..681091a68 100644 --- 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,6 +660,7 @@ - (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 @@ -668,24 +668,23 @@ - (CALayer *)layer 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 +1032,7 @@ - (void)__layout BOOL loaded = NO; { - ASDN::MutexLocker l(__instanceLock__); + ASDN::UniqueLock l(__instanceLock__); loaded = [self _locked_isNodeLoaded]; CGRect bounds = _threadSafeBounds; @@ -1055,10 +1054,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]; } @@ -3546,15 +3544,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 index 1ace8c74b..bed665560 100644 --- a/Source/ASImageNode.mm +++ b/Source/ASImageNode.mm @@ -434,12 +434,12 @@ + (UIImage *)displayWithParameters:(id)parameter isCancelled:(NS_NOESC static ASWeakMap *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 index 9feb18afd..2c679de18 100644 --- 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 *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 index 4cb4ba8c5..f61ef5199 100644 --- a/Source/ASVideoNode.mm +++ b/Source/ASVideoNode.mm @@ -10,6 +10,7 @@ #import #import #import +#import #import #import #import @@ -322,6 +323,7 @@ - (void)setVideoPlaceholderImage:(UIImage *)image - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context { + ASDN::UniqueLock l(__instanceLock__); ASLockScopeSelf(); if (object == _currentPlayerItem) { @@ -330,8 +332,9 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N 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 +357,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 +378,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 index 24ccf95bf..c13fc728b 100644 --- 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 index 42d937ad4..3a5fa00f8 100644 --- 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); }; diff --git a/Source/Details/ASThread.h b/Source/Details/ASThread.h index bdcfd2618..a472766d6 100644 --- a/Source/Details/ASThread.h +++ b/Source/Details/ASThread.h @@ -10,6 +10,7 @@ #import #import +#import #import #import #import @@ -143,106 +144,6 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) namespace ASDN { - template - class Locker - { - T &_l; - -#if TIME_LOCKER - CFTimeInterval _ti; - const char *_name; -#endif - - public: -#if !TIME_LOCKER - - Locker (T &l) noexcept : _l (l) { - _l.lock (); - } - - ~Locker () { - _l.unlock (); - } - - // non-copyable. - Locker(const Locker&) = delete; - Locker &operator=(const Locker&) = delete; - -#else - - Locker (T &l, const char *name = NULL) noexcept : _l (l), _name(name) { - _ti = CACurrentMediaTime(); - _l.lock (); - } - - ~Locker () { - _l.unlock (); - if (_name) { - printf(_name, NULL); - printf(" dt:%f\n", CACurrentMediaTime() - _ti); - } - } - -#endif - - }; - - template - class SharedLocker - { - std::shared_ptr _l; - -#if TIME_LOCKER - CFTimeInterval _ti; - const char *_name; -#endif - - public: -#if !TIME_LOCKER - - SharedLocker (std::shared_ptr const& l) noexcept : _l (l) { - ASDisplayNodeCAssertTrue(_l != nullptr); - _l->lock (); - } - - ~SharedLocker () { - _l->unlock (); - } - - // non-copyable. - SharedLocker(const SharedLocker&) = delete; - SharedLocker &operator=(const SharedLocker&) = delete; - -#else - - SharedLocker (std::shared_ptr const& l, const char *name = NULL) noexcept : _l (l), _name(name) { - _ti = CACurrentMediaTime(); - _l->lock (); - } - - ~SharedLocker () { - _l->unlock (); - if (_name) { - printf(_name, NULL); - printf(" dt:%f\n", CACurrentMediaTime() - _ti); - } - } - -#endif - - }; - - template - class Unlocker - { - T &_l; - public: - Unlocker (T &l) noexcept : _l (l) { _l.unlock (); } - ~Unlocker () {_l.lock ();} - Unlocker(Unlocker&) = delete; - Unlocker &operator=(Unlocker&) = delete; - }; - // Set once in Mutex constructor. Linker fails if this is a member variable. ?? static BOOL gMutex_unfair; @@ -414,41 +315,8 @@ namespace ASDN { RecursiveMutex () : Mutex (true) {} }; - typedef Locker MutexLocker; - typedef SharedLocker MutexSharedLocker; - typedef Unlocker MutexUnlocker; - - /** - If you are creating a static mutex, use StaticMutex. This avoids expensive constructor overhead at startup (or worse, ordering - issues between different static objects). It also avoids running a destructor on app exit time (needless expense). - - Note that you can, but should not, use StaticMutex for non-static objects. It will leak its mutex on destruction, - so avoid that! - */ - struct StaticMutex - { - StaticMutex () : _m (PTHREAD_MUTEX_INITIALIZER) {} - - // non-copyable. - StaticMutex(const StaticMutex&) = delete; - StaticMutex &operator=(const StaticMutex&) = delete; - - void lock () { - AS_POSIX_ASSERT_NOERR(pthread_mutex_lock (this->mutex())); - } - - void unlock () { - AS_POSIX_ASSERT_NOERR(pthread_mutex_unlock (this->mutex())); - } - - pthread_mutex_t *mutex () { return &_m; } - - private: - pthread_mutex_t _m; - }; - - typedef Locker StaticMutexLocker; - typedef Unlocker StaticMutexUnlocker; + typedef std::lock_guard MutexLocker; + typedef std::unique_lock UniqueLock; } // namespace ASDN diff --git a/Source/Private/ASLayoutTransition.mm b/Source/Private/ASLayoutTransition.mm index 1e482afa6..b7d9cda44 100644 --- a/Source/Private/ASLayoutTransition.mm +++ b/Source/Private/ASLayoutTransition.mm @@ -81,7 +81,7 @@ - (instancetype)initWithNode:(ASDisplayNode *)node - (BOOL)isSynchronous { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); return !ASLayoutCanTransitionAsynchronous(_pendingLayout.layout); } @@ -93,7 +93,7 @@ - (void)commitTransition - (void)applySubnodeInsertionsAndMoves { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); [self calculateSubnodeOperationsIfNeeded]; // Create an activity even if no subnodes affected. @@ -131,7 +131,7 @@ - (void)applySubnodeInsertionsAndMoves - (void)applySubnodeRemovals { as_activity_scope(as_activity_create("Apply subnode removals", AS_ACTIVITY_CURRENT, OS_ACTIVITY_FLAG_DEFAULT)); - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); [self calculateSubnodeOperationsIfNeeded]; if (_removedSubnodes.count == 0) { @@ -151,7 +151,7 @@ - (void)applySubnodeRemovals - (void)calculateSubnodeOperationsIfNeeded { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); if (_calculatedSubnodeOperations) { return; } @@ -206,27 +206,27 @@ - (void)calculateSubnodeOperationsIfNeeded - (NSArray *)currentSubnodesWithTransitionContext:(_ASTransitionContext *)context { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); return _node.subnodes; } - (NSArray *)insertedSubnodesWithTransitionContext:(_ASTransitionContext *)context { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); [self calculateSubnodeOperationsIfNeeded]; return _insertedSubnodes; } - (NSArray *)removedSubnodesWithTransitionContext:(_ASTransitionContext *)context { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); [self calculateSubnodeOperationsIfNeeded]; return _removedSubnodes; } - (ASLayout *)transitionContext:(_ASTransitionContext *)context layoutForKey:(NSString *)key { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); if ([key isEqualToString:ASTransitionContextFromLayoutKey]) { return _previousLayout.layout; } else if ([key isEqualToString:ASTransitionContextToLayoutKey]) { @@ -238,7 +238,7 @@ - (ASLayout *)transitionContext:(_ASTransitionContext *)context layoutForKey:(NS - (ASSizeRange)transitionContext:(_ASTransitionContext *)context constrainedSizeForKey:(NSString *)key { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); if ([key isEqualToString:ASTransitionContextFromLayoutKey]) { return _previousLayout.constrainedSize; } else if ([key isEqualToString:ASTransitionContextToLayoutKey]) { diff --git a/Source/TextKit/ASTextKitContext.mm b/Source/TextKit/ASTextKitContext.mm index 0995a66f6..fa0fb8834 100644 --- a/Source/TextKit/ASTextKitContext.mm +++ b/Source/TextKit/ASTextKitContext.mm @@ -32,9 +32,9 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString { if (self = [super init]) { // Concurrently initialising TextKit components crashes (rdar://18448377) so we use a global lock. - // Allocate __staticMutex on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136) - static ASDN::StaticMutex& __staticMutex = *new ASDN::StaticMutex; - ASDN::StaticMutexLocker l(__staticMutex); + // Allocate mutex on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136) + static auto *mutex = new ASDN::Mutex; + ASDN::MutexLocker l(*mutex); __instanceLock__ = std::make_shared(); @@ -66,7 +66,7 @@ - (void)performBlockWithLockedTextKitComponents:(NS_NOESCAPE void (^)(NSLayoutMa NSTextStorage *, NSTextContainer *))block { - ASDN::MutexSharedLocker l(__instanceLock__); + ASDN::MutexLocker l(*__instanceLock__); if (block) { block(_layoutManager, _textStorage, _textContainer); } From dc49c6688cd3d11d34b02f2528301de261bcd47b Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Nov 2018 09:51:26 -0800 Subject: [PATCH 2/5] Remove extra line --- Source/ASVideoNode.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/ASVideoNode.mm b/Source/ASVideoNode.mm index f61ef5199..fe368a12c 100644 --- a/Source/ASVideoNode.mm +++ b/Source/ASVideoNode.mm @@ -324,7 +324,6 @@ - (void)setVideoPlaceholderImage:(UIImage *)image - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context { ASDN::UniqueLock l(__instanceLock__); - ASLockScopeSelf(); if (object == _currentPlayerItem) { if ([keyPath isEqualToString:kStatus]) { From 1d79d46ed704b314c2541a48600f3a3862aaef44 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Nov 2018 10:04:29 -0800 Subject: [PATCH 3/5] Kick the CI --- Source/Details/ASThread.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Source/Details/ASThread.h b/Source/Details/ASThread.h index a472766d6..66a08dfc1 100644 --- a/Source/Details/ASThread.h +++ b/Source/Details/ASThread.h @@ -96,7 +96,6 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) #ifdef __cplusplus -#define TIME_LOCKER 0 /** * Enable this flag to collect information on the owning thread and ownership level of a mutex. * These properties are useful to determine if a mutex has been acquired and in case of a recursive mutex, how many times that happened. @@ -113,10 +112,6 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) #define CHECK_LOCKING_SAFETY 0 #endif -#if TIME_LOCKER -#import -#endif - #include // This MUST always execute, even when assertions are disabled. Otherwise all lock operations become no-ops! From c6f0cf199e9e8e8b3a1f0cb28c8d735a34e70054 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Nov 2018 11:03:31 -0800 Subject: [PATCH 4/5] Move C++ down --- Source/Details/ASThread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Details/ASThread.h b/Source/Details/ASThread.h index 66a08dfc1..701e6c10a 100644 --- a/Source/Details/ASThread.h +++ b/Source/Details/ASThread.h @@ -10,7 +10,6 @@ #import #import -#import #import #import #import @@ -113,6 +112,7 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) #endif #include +#include // This MUST always execute, even when assertions are disabled. Otherwise all lock operations become no-ops! // (To be explicit, do not turn this into an NSAssert, assert(), or any other kind of statement where the From e6e0f3e0b84e71796f615397aad34cd89117295a Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Nov 2018 11:56:19 -0800 Subject: [PATCH 5/5] Fix thing --- Source/ASDisplayNode.mm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 681091a68..892376265 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -667,7 +667,6 @@ - (CALayer *)layer // 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. l.unlock(); [self __setNeedsLayout]; l.lock(); @@ -1119,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: @@ -1136,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); }