ThePain/Swiftgram
1
0
Fork 0
mirror of https://github.com/Swiftgram/Telegram-iOS.git synced 2026-01-06 13:12:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix deadlock when laying out on multiple threads

Browse Source 
Summary:
We observed a deadlock which occurred when two threads were laying out the same set of nodes.

On one thread, layout would occur on a leaf node. It would lock and as part of this layout
process, ASDK walks up the node tree and calls __setNeedsLayout on its supernode until it
reaches the supernode with no supernode. When the supernode gets its call to __setNeedsLayout
it also locks. So leaf node locks and then awaits supernode lock.

On another thread, we're doing a layout pass on the supernode in the above thread. This locks
the supernode and attempts to lock the leaf node. This deadlocks (remember the above thread
is holding onto the leaf lock and awaiting the supernode lock. This thread is holding onto
the supernode lock and awaiting the leaf lock).

This is all exacerbated by the use of recursive locks.
This commit is contained in:
Garrett Moon
2016-05-03 15:56:38 -07:00
parent 083bc9d028
commit 938ecd9b6f
3 changed files with 30 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
AsyncDisplayKit/ASCellNode.mm
View File

@@ -125,6 +125,8 @@
{
CGSize oldSize = self.calculatedSize;
[super __setNeedsLayout];
ASDN::MutexLocker l(_propertyLock);
[self didRelayoutFromOldSize:oldSize toNewSize:self.calculatedSize];
}

38
AsyncDisplayKit/ASDisplayNode.mm
View File

@@ -1001,27 +1001,31 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
[self invalidateCalculatedLayout];
if (_supernode) {
ASDisplayNode *supernode = _supernode;
ASDN::MutexUnlocker u(_propertyLock);
// Cause supernode's layout to be invalidated
// We need to release the lock to prevent a deadlock
[_supernode setNeedsLayout];
} else {
// This is the root node. Trigger a full measurement pass on *current* thread. Old constrained size is re-used.
[self measureWithSizeRange:oldConstrainedSize];
return;
}
// This is the root node. Trigger a full measurement pass on *current* thread. Old constrained size is re-used.
[self measureWithSizeRange:oldConstrainedSize];
CGRect oldBounds = self.bounds;
CGSize oldSize = oldBounds.size;
CGSize newSize = _layout.size;
CGRect oldBounds = self.bounds;
CGSize oldSize = oldBounds.size;
CGSize newSize = _layout.size;
if (! CGSizeEqualToSize(oldSize, newSize)) {
self.bounds = (CGRect){ oldBounds.origin, newSize };
if (! CGSizeEqualToSize(oldSize, newSize)) {
self.bounds = (CGRect){ oldBounds.origin, newSize };
// Frame's origin must be preserved. Since it is computed from bounds size, anchorPoint
// and position (see frame setter in ASDisplayNode+UIViewBridge), position needs to be adjusted.
CGPoint anchorPoint = self.anchorPoint;
CGPoint oldPosition = self.position;
CGFloat xDelta = (newSize.width - oldSize.width) * anchorPoint.x;
CGFloat yDelta = (newSize.height - oldSize.height) * anchorPoint.y;
self.position = CGPointMake(oldPosition.x + xDelta, oldPosition.y + yDelta);
}
// Frame's origin must be preserved. Since it is computed from bounds size, anchorPoint
// and position (see frame setter in ASDisplayNode+UIViewBridge), position needs to be adjusted.
CGPoint anchorPoint = self.anchorPoint;
CGPoint oldPosition = self.position;
CGFloat xDelta = (newSize.width - oldSize.width) * anchorPoint.x;
CGFloat yDelta = (newSize.height - oldSize.height) * anchorPoint.y;
self.position = CGPointMake(oldPosition.x + xDelta, oldPosition.y + yDelta);
}
}

7
AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm
View File

@@ -37,6 +37,7 @@
#if DISPLAYNODE_USE_LOCKS
#define _bridge_prologue_read ASDN::MutexLocker l(_propertyLock); ASDisplayNodeAssertThreadAffinity(self)
#define _bridge_prologue_write ASDN::MutexLocker l(_propertyLock)
#define _bridge_prologue_write_unlock ASDN::MutexUnlocker u(_propertyLock)
#else
#define _bridge_prologue_read ASDisplayNodeAssertThreadAffinity(self)
#define _bridge_prologue_write
@@ -331,7 +332,11 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNo
// The node is loaded and we're on main.
// Quite the opposite of setNeedsDisplay, we must call __setNeedsLayout before messaging
// the view or layer to ensure that measurement and implicitly added subnodes have been handled.
// Calling __setNeedsLayout while holding the property lock can cause deadlocks
_bridge_prologue_write_unlock;
[self __setNeedsLayout];
_bridge_prologue_write;
_messageToViewOrLayer(setNeedsLayout);
} else if (__loaded(self)) {
// The node is loaded but we're not on main.
@@ -341,7 +346,9 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNo
[ASDisplayNodeGetPendingState(self) setNeedsLayout];
} else {
// The node is not loaded and we're not on main.
_bridge_prologue_write_unlock;
[self __setNeedsLayout];
_bridge_prologue_write;
}
}