Skip to content

Commit

Permalink
Revert - ASThread: Remove Locker, Unlocker, and SharedMutex (TextureG…
Browse files Browse the repository at this point in the history
  • Loading branch information
ernestmama committed Nov 14, 2018
1 parent ce78fa0 commit 88c3463
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 76 deletions.
73 changes: 37 additions & 36 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,12 @@ - (ASDisplayNodeMethodOverrides)methodOverrides

- (void)onDidLoad:(ASDisplayNodeDidLoadBlock)body
{
ASDN::UniqueLock l(__instanceLock__);
ASDN::MutexLocker l(__instanceLock__);

if ([self _locked_isNodeLoaded]) {
ASDisplayNodeAssertThreadAffinity(self);
l.unlock();
ASDN::MutexUnlocker l(__instanceLock__);
body(self);
return;
} else if (_onDidLoadBlocks == nil) {
_onDidLoadBlocks = [NSMutableArray arrayWithObject:body];
} else {
Expand Down Expand Up @@ -602,7 +601,7 @@ - (BOOL)_locked_isNodeLoaded

- (UIView *)view
{
ASDN::UniqueLock l(__instanceLock__);
ASDN::MutexLocker l(__instanceLock__);

ASDisplayNodeAssert(!_flags.layerBacked, @"Call to -view undefined on layer-backed nodes");
BOOL isLayerBacked = _flags.layerBacked;
Expand All @@ -627,28 +626,30 @@ - (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.
l.unlock();
// MutexUnlocker will re-lock at the end of scope.
ASDN::MutexUnlocker u(__instanceLock__);
[self __setNeedsLayout];
l.lock();
}

[self _locked_applyPendingStateToViewOrLayer];

// The following methods should not be called with a lock
l.unlock();
{
// The following methods should not be called with a lock
ASDN::MutexUnlocker u(__instanceLock__);

// 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::UniqueLock l(__instanceLock__);
ASDN::MutexLocker l(__instanceLock__);
if (_layer != nil) {
return _layer;
}
Expand All @@ -660,30 +661,31 @@ - (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.
l.unlock();
// MutexUnlocker will re-lock at the end of scope.
ASDN::MutexUnlocker u(__instanceLock__);
[self __setNeedsLayout];
l.lock();
}

[self _locked_applyPendingStateToViewOrLayer];

// The following methods should not be called with a lock
l.unlock();
{
// The following methods should not be called with a lock
ASDN::MutexUnlocker u(__instanceLock__);

// 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.
Expand Down Expand Up @@ -1031,7 +1033,7 @@ - (void)__layout

BOOL loaded = NO;
{
ASDN::UniqueLock l(__instanceLock__);
ASDN::MutexLocker l(__instanceLock__);
loaded = [self _locked_isNodeLoaded];
CGRect bounds = _threadSafeBounds;

Expand All @@ -1053,9 +1055,10 @@ - (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).
l.unlock();
[self _u_measureNodeWithBoundsIfNecessary:bounds];
l.lock();
{
ASDN::MutexUnlocker u(__instanceLock__);
[self _u_measureNodeWithBoundsIfNecessary:bounds];
}

[self _locked_layoutPlaceholderIfNecessary];
}
Expand Down Expand Up @@ -1118,7 +1121,7 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
{
__ASDisplayNodeCheckForLayoutMethodOverrides;

ASDN::UniqueLock l(__instanceLock__);
ASDN::MutexLocker l(__instanceLock__);

#if YOGA
// There are several cases where Yoga could arrive here:
Expand All @@ -1135,13 +1138,11 @@ - (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);
}
Expand Down Expand Up @@ -3539,15 +3540,15 @@ - (void)applyPendingViewState
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);

ASDN::UniqueLock l(__instanceLock__);
ASDN::MutexLocker 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.
l.unlock();
// MutexUnlocker will re-lock at the end of scope.
ASDN::MutexUnlocker u(__instanceLock__);
[self __setNeedsLayout];
l.lock();
}

[self _locked_applyPendingViewState];
Expand Down
6 changes: 3 additions & 3 deletions Source/ASImageNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 auto *cacheLock = new ASDN::Mutex;
static ASDN::StaticMutex& cacheLock = *new ASDN::StaticMutex;

+ (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:(id)drawParameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled
{
{
ASDN::MutexLocker l(*cacheLock);
ASDN::StaticMutexLocker l(cacheLock);
if (!cache) {
cache = [[ASWeakMap alloc] init];
}
Expand All @@ -456,7 +456,7 @@ + (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:
}

{
ASDN::MutexLocker l(*cacheLock);
ASDN::StaticMutexLocker l(cacheLock);
return [cache setObject:contents forKey:key];
}
}
Expand Down
6 changes: 3 additions & 3 deletions Source/ASTextNode2.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 auto *layoutCacheLock = new ASDN::Mutex;
static ASDN::StaticMutex& layoutCacheLock = *new ASDN::StaticMutex;
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) {
Expand All @@ -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 };
{
Expand Down
12 changes: 3 additions & 9 deletions Source/ASVideoNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#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>
Expand Down Expand Up @@ -323,17 +322,16 @@ - (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]) {
if ([change[NSKeyValueChangeNewKey] integerValue] == AVPlayerItemStatusReadyToPlay) {
if (self.playerState != ASVideoNodePlayerStatePlaying) {
self.playerState = ASVideoNodePlayerStateReadyToPlay;
if (_shouldBePlaying && ASInterfaceStateIncludesVisible(self.interfaceState)) {
l.unlock();
ASUnlockScope(self);
[self play];
l.lock();
}
}
// If we don't yet have a placeholder image update it now that we should have data available for it
Expand All @@ -356,10 +354,8 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
[self.delegate videoNodeDidRecoverFromStall:self];
}

l.unlock();
ASUnlockScope(self);
[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)) {
Expand All @@ -377,8 +373,6 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
}
}
}

// NOTE: Early return above.
}

- (void)tapped
Expand Down
6 changes: 3 additions & 3 deletions Source/Details/ASBasicImageDownloader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 auto *currentRequestsLock = new ASDN::Mutex;
static ASDN::StaticMutex& currentRequestsLock = *new ASDN::StaticMutex;

+ (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL
{
ASDN::MutexLocker l(*currentRequestsLock);
ASDN::StaticMutexLocker l(currentRequestsLock);
if (!currentRequests) {
currentRequests = [[NSMutableDictionary alloc] init];
}
Expand All @@ -57,7 +57,7 @@ + (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL

+ (void)cancelContextWithURL:(NSURL *)URL
{
ASDN::MutexLocker l(*currentRequestsLock);
ASDN::StaticMutexLocker l(currentRequestsLock);
if (currentRequests) {
[currentRequests removeObjectForKey:URL];
}
Expand Down
11 changes: 4 additions & 7 deletions Source/Details/ASMainSerialQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,19 @@ - (NSUInteger)numberOfScheduledBlocks

- (void)performBlockOnMainThread:(dispatch_block_t)block
{

ASDN::UniqueLock l(_serialQueueLock);
ASDN::MutexLocker l(_serialQueueLock);
[_blocks addObject:block];
{
l.unlock();
ASDN::MutexUnlocker u(_serialQueueLock);
[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];
Expand All @@ -63,9 +61,8 @@ - (void)runBlocks
break;
}
{
l.unlock();
ASDN::MutexUnlocker u(self->_serialQueueLock);
block();
l.lock();
}
} while (true);
};
Expand Down
Loading

0 comments on commit 88c3463

Please sign in to comment.