Remove the lock for accessing _pendingDisplayNodes and force methods regarding display to the main thread

This should fix a deadlock in ASDisplayNode and it's caused by displayWillStart: and where one thread is recursing down the tree and another thread is recursing up the tree. We remove the lock in _pendingDisplayNodes for the property, but need to guarantee that we only modify the _pendingDisplayNodes state on the main thread.

Furthermore add documentation on what thread displayWillStart and displayDidFinish will be called
This commit is contained in:
Michael Schneider
2016-03-15 16:30:08 -07:00
parent f97a509541
commit 863b0ca956
2 changed files with 30 additions and 21 deletions

View File

@@ -206,6 +206,8 @@ NS_ASSUME_NONNULL_BEGIN
*
* @discussion Subclasses may override this method to be notified when display (asynchronous or synchronous) is
* about to begin.
*
* @note Called on the main thread only
*/
- (void)displayWillStart ASDISPLAYNODE_REQUIRES_SUPER;
@@ -214,6 +216,8 @@ NS_ASSUME_NONNULL_BEGIN
*
* @discussion Subclasses may override this method to be notified when display (asynchronous or synchronous) has
* completed.
*
* @note Called on the main thread only
*/
- (void)displayDidFinish ASDISPLAYNODE_REQUIRES_SUPER;

View File

@@ -1657,7 +1657,7 @@ static NSInteger incrementIfFound(NSInteger i) {
// The node sending the message should usually be passed as the parameter, similar to the delegation pattern.
- (void)_pendingNodeWillDisplay:(ASDisplayNode *)node
{
ASDN::MutexLocker l(_propertyLock);
ASDisplayNodeAssertMainThread();
if (!_pendingDisplayNodes) {
_pendingDisplayNodes = [[NSMutableSet alloc] init];
@@ -1670,27 +1670,25 @@ static NSInteger incrementIfFound(NSInteger i) {
// The node sending the message should usually be passed as the parameter, similar to the delegation pattern.
- (void)_pendingNodeDidDisplay:(ASDisplayNode *)node
{
ASDN::MutexLocker l(_propertyLock);
ASDisplayNodeAssertMainThread();
[_pendingDisplayNodes removeObject:node];
// only trampoline if there is a placeholder and nodes are done displaying
if ([self _pendingDisplayNodesHaveFinished] && _placeholderLayer.superlayer) {
dispatch_async(dispatch_get_main_queue(), ^{
void (^cleanupBlock)() = ^{
[self _tearDownPlaceholderLayer];
};
void (^cleanupBlock)() = ^{
[self _tearDownPlaceholderLayer];
};
if (_placeholderFadeDuration > 0.0 && ASInterfaceStateIncludesVisible(self.interfaceState)) {
[CATransaction begin];
[CATransaction setCompletionBlock:cleanupBlock];
[CATransaction setAnimationDuration:_placeholderFadeDuration];
_placeholderLayer.opacity = 0.0;
[CATransaction commit];
} else {
cleanupBlock();
}
});
if (_placeholderFadeDuration > 0.0 && ASInterfaceStateIncludesVisible(self.interfaceState)) {
[CATransaction begin];
[CATransaction setCompletionBlock:cleanupBlock];
[CATransaction setAnimationDuration:_placeholderFadeDuration];
_placeholderLayer.opacity = 0.0;
[CATransaction commit];
} else {
cleanupBlock();
}
}
}
@@ -2260,6 +2258,8 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)displayWillStart
{
ASDisplayNodeAssertMainThread();
// in case current node takes longer to display than it's subnodes, treat it as a dependent node
[self _pendingNodeWillDisplay:self];
@@ -2288,6 +2288,8 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)displayDidFinish
{
ASDisplayNodeAssertMainThread();
[self _pendingNodeDidDisplay:self];
[_supernode subnodeDisplayDidFinish:self];
@@ -2494,11 +2496,14 @@ static void _recursivelySetDisplaySuspended(ASDisplayNode *node, CALayer *layer,
self.asyncLayer.displaySuspended = flag;
if ([self __implementsDisplay]) {
if (flag) {
[_supernode subnodeDisplayDidFinish:self];
} else {
[_supernode subnodeDisplayWillStart:self];
}
// Display start and finish methods needs to happen on the main thread
ASPerformBlockOnMainThread(^{
if (flag) {
[_supernode subnodeDisplayDidFinish:self];
} else {
[_supernode subnodeDisplayWillStart:self];
}
});
}
}