Skip to content

Commit 704d2ad

Browse files
nguyenhuyAdlai Holler
authored andcommitted
[ASDisplayNode] Avoid holding instance lock while calling layout subclass hooks (facebookarchive#3064)
* Fix deadlock occurs during layout of ASDisplayNode that triggered a change set update in ASDataController * Access _transitionInProgress ivar directly if already locked
1 parent 5b31cd6 commit 704d2ad

File tree

1 file changed

+30
-23
lines changed

1 file changed

+30
-23
lines changed

AsyncDisplayKit/ASDisplayNode.mm

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
* and check ownership count of the mutex.
4747
*/
4848
#if CHECK_LOCKING_SAFETY
49-
#define ASDisplayNodeAssertLockUnownedByCurrentThread(lock) ASDisplayNodeAssertFalse(lock.ownedByCurrentThread());
49+
#define ASDisplayNodeAssertLockUnownedByCurrentThread(lock) ASDisplayNodeAssertFalse(lock.ownedByCurrentThread())
5050
#else
5151
#define ASDisplayNodeAssertLockUnownedByCurrentThread(lock)
5252
#endif
@@ -924,29 +924,33 @@ - (void)invalidateCalculatedLayout
924924
- (void)__layout
925925
{
926926
ASDisplayNodeAssertMainThread();
927-
ASDN::MutexLocker l(__instanceLock__);
928-
CGRect bounds = _threadSafeBounds;
929-
930-
if (CGRectEqualToRect(bounds, CGRectZero)) {
931-
// Performing layout on a zero-bounds view often results in frame calculations
932-
// with negative sizes after applying margins, which will cause
933-
// measureWithSizeRange: on subnodes to assert.
934-
LOG(@"Warning: No size given for node before node was trying to layout itself: %@. Please provide a frame for the node.", self);
935-
return;
936-
}
927+
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
937928

938-
// If a current layout transition is in progress there is no need to do a measurement and layout pass in here as
939-
// this is supposed to happen within the layout transition process
940-
if ([self _isTransitionInProgress]) {
941-
return;
942-
}
929+
{
930+
ASDN::MutexLocker l(__instanceLock__);
931+
CGRect bounds = _threadSafeBounds;
943932

944-
// This method will confirm that the layout is up to date (and update if needed).
945-
// Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
946-
[self _locked_measureNodeWithBoundsIfNecessary:bounds];
947-
_pendingDisplayNodeLayout = nullptr;
948-
949-
[self _locked_layoutPlaceholderIfNecessary];
933+
if (CGRectEqualToRect(bounds, CGRectZero)) {
934+
// Performing layout on a zero-bounds view often results in frame calculations
935+
// with negative sizes after applying margins, which will cause
936+
// measureWithSizeRange: on subnodes to assert.
937+
LOG(@"Warning: No size given for node before node was trying to layout itself: %@. Please provide a frame for the node.", self);
938+
return;
939+
}
940+
941+
// If a current layout transition is in progress there is no need to do a measurement and layout pass in here as
942+
// this is supposed to happen within the layout transition process
943+
if (_transitionInProgress) {
944+
return;
945+
}
946+
947+
// This method will confirm that the layout is up to date (and update if needed).
948+
// Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
949+
[self _locked_measureNodeWithBoundsIfNecessary:bounds];
950+
_pendingDisplayNodeLayout = nullptr;
951+
952+
[self _locked_layoutPlaceholderIfNecessary];
953+
}
950954

951955
[self layout];
952956
[self layoutDidFinish];
@@ -1062,6 +1066,8 @@ - (ASSizeRange)_locked_constrainedSizeForLayoutPass
10621066
- (void)layoutDidFinish
10631067
{
10641068
// Hook for subclasses
1069+
ASDisplayNodeAssertMainThread();
1070+
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
10651071
}
10661072

10671073
#pragma mark Calculation
@@ -1316,6 +1322,7 @@ - (void)_locked_displayNodeDidInvalidateSizeNewSize:(CGSize)size
13161322
- (void)layout
13171323
{
13181324
ASDisplayNodeAssertMainThread();
1325+
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
13191326

13201327
__instanceLock__.lock();
13211328
if (_calculatedDisplayNodeLayout->isDirty()) {
@@ -1488,7 +1495,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize
14881495
- (void)cancelLayoutTransition
14891496
{
14901497
ASDN::MutexLocker l(__instanceLock__);
1491-
if ([self _isTransitionInProgress]) {
1498+
if (_transitionInProgress) {
14921499
// Cancel transition in progress
14931500
[self _finishOrCancelTransition];
14941501

0 commit comments

Comments
 (0)