diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e9b667d2..c91ecbf5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - [IGListKit] Adds missing UIScrollViewDelegate method to DataSource proxy [Sergey Pronin](https://github.com/wannabehero) - Fix misleading/scary stack trace shown when an assertion occurs during node measurement [Huy Nguyen](https://github.com/nguyenhuy) [#1022](https://github.com/TextureGroup/Texture/pull/1022) - Fix build on 32-bit simulator in Xcode 9.3 and later, caused by `Thread-local storage is not supported on this architecture.` [Adlai Holler](https://github.com/Adlai-Holler) +- Enable locking assertions (and add some more) to improve and enforce locking safety within the framework [Huy Nguyen](https://github.com/nguyenhuy) [#1024](https://github.com/TextureGroup/Texture/pull/1024) ## 2.7 - Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index e77c354ab..f6179260d 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -184,6 +184,7 @@ - (ASSizeRange)constrainedSizeForCalculatedLayout - (ASSizeRange)_locked_constrainedSizeForCalculatedLayout { + ASAssertLocked(__instanceLock__); if (_pendingDisplayNodeLayout != nullptr && _pendingDisplayNodeLayout->isValid(_layoutVersion)) { return _pendingDisplayNodeLayout->constrainedSize; } @@ -218,9 +219,10 @@ @implementation ASDisplayNode (ASLayoutInternal) */ - (void)_u_setNeedsLayoutFromAbove { - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock); - as_activity_create_for_scope("Set needs layout from above"); ASDisplayNodeAssertThreadAffinity(self); + ASAssertUnlocked(__instanceLock__); + + as_activity_create_for_scope("Set needs layout from above"); // Mark the node for layout in the next layout pass [self setNeedsLayout]; @@ -243,7 +245,7 @@ - (void)_u_setNeedsLayoutFromAbove - (void)_rootNodeDidInvalidateSize { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); __instanceLock__.lock(); @@ -273,7 +275,7 @@ - (void)_rootNodeDidInvalidateSize - (void)displayNodeDidInvalidateSizeNewSize:(CGSize)size { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); // The default implementation of display node changes the size of itself to the new size CGRect oldBounds = self.bounds; @@ -295,19 +297,19 @@ - (void)displayNodeDidInvalidateSizeNewSize:(CGSize)size - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds { - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock); + ASAssertUnlocked(__instanceLock__); BOOL isInLayoutPendingState = NO; { ASDN::MutexLocker l(__instanceLock__); // Check if we are a subnode in a layout transition. // In this case no measurement is needed as it's part of the layout transition - if ([self _isLayoutTransitionInvalid]) { + if ([self _locked_isLayoutTransitionInvalid]) { return; } - + CGSize boundsSizeForLayout = ASCeilSizeValues(bounds.size); - + // Prefer a newer and not yet applied _pendingDisplayNodeLayout over _calculatedDisplayNodeLayout // If there is no such _pending, check if _calculated is valid to reuse (avoiding recalculation below). BOOL pendingLayoutIsPreferred = NO; @@ -328,12 +330,16 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds if (!pendingLayoutIsPreferred && calculatedLayoutIsReusable) { return; } - + as_activity_create_for_scope("Update node layout for current bounds"); - as_log_verbose(ASLayoutLog(), "Node %@, bounds size %@, calculatedSize %@, calculatedIsDirty %d", self, NSStringFromCGSize(boundsSizeForLayout), NSStringFromCGSize(_calculatedDisplayNodeLayout->layout.size), _calculatedDisplayNodeLayout->version < _layoutVersion.load()); + as_log_verbose(ASLayoutLog(), "Node %@, bounds size %@, calculatedSize %@, calculatedIsDirty %d", + self, + NSStringFromCGSize(boundsSizeForLayout), + NSStringFromCGSize(_calculatedDisplayNodeLayout->layout.size), + _calculatedDisplayNodeLayout->version < _layoutVersion); // _calculatedDisplayNodeLayout is not reusable we need to transition to a new one [self cancelLayoutTransition]; - + BOOL didCreateNewContext = NO; ASLayoutElementContext *context = ASLayoutElementGetCurrentContext(); if (context == nil) { @@ -341,17 +347,17 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds ASLayoutElementPushContext(context); didCreateNewContext = YES; } - + // Figure out previous and pending layouts for layout transition std::shared_ptr nextLayout = _pendingDisplayNodeLayout; -#define layoutSizeDifferentFromBounds !CGSizeEqualToSize(nextLayout->layout.size, boundsSizeForLayout) - + #define layoutSizeDifferentFromBounds !CGSizeEqualToSize(nextLayout->layout.size, boundsSizeForLayout) + // nextLayout was likely created by a call to layoutThatFits:, check if it is valid and can be applied. // If our bounds size is different than it, or invalid, recalculate. Use #define to avoid nullptr-> BOOL pendingLayoutApplicable = NO; if (nextLayout == nullptr) { as_log_verbose(ASLayoutLog(), "No pending layout."); - } else if (nextLayout->version < _layoutVersion) { + } else if (nextLayout->isValid(_layoutVersion) == NO) { as_log_verbose(ASLayoutLog(), "Pending layout is stale."); } else if (layoutSizeDifferentFromBounds) { as_log_verbose(ASLayoutLog(), "Pending layout size %@ doesn't match bounds size.", NSStringFromCGSize(nextLayout->layout.size)); @@ -359,7 +365,7 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds as_log_verbose(ASLayoutLog(), "Using pending layout %@.", nextLayout->layout); pendingLayoutApplicable = YES; } - + if (!pendingLayoutApplicable) { as_log_verbose(ASLayoutLog(), "Measuring with previous constrained size."); // Use the last known constrainedSize passed from a parent during layout (if never, use bounds). @@ -373,11 +379,11 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds // Release it and any orphaned subnodes it retains _pendingDisplayNodeLayout = nullptr; } - + if (didCreateNewContext) { ASLayoutElementPopContext(); } - + // If our new layout's desired size for self doesn't match current size, ask our parent to update it. // This can occur for either pre-calculated or newly-calculated layouts. if (nextLayout->requestedLayoutFromAbove == NO @@ -390,14 +396,17 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds // In this case, we need to detect that we've already asked to be resized to match this // particular ASLayout object, and shouldn't loop asking again unless we have a different ASLayout. nextLayout->requestedLayoutFromAbove = YES; - __instanceLock__.unlock(); - [self _u_setNeedsLayoutFromAbove]; - __instanceLock__.lock(); + + { + ASDN::MutexUnlocker u(__instanceLock__); + [self _u_setNeedsLayoutFromAbove]; + } + // Update the layout's version here because _u_setNeedsLayoutFromAbove calls __setNeedsLayout which in turn increases _layoutVersion // Failing to do this will cause the layout to be invalid immediately nextLayout->version = _layoutVersion; } - + // Prepare to transition to nextLayout ASDisplayNodeAssertNotNil(nextLayout->layout, @"nextLayout->layout should not be nil! %@", self); _pendingLayoutTransition = [[ASLayoutTransition alloc] initWithNode:self @@ -405,7 +414,7 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds previousLayout:_calculatedDisplayNodeLayout]; isInLayoutPendingState = ASHierarchyStateIncludesLayoutPending(_hierarchyState); } - + // If a parent is currently executing a layout transition, perform our layout application after it. if (isInLayoutPendingState == NO) { // If no transition, apply our new layout immediately (common case). @@ -413,18 +422,26 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds } } +- (ASSizeRange)_constrainedSizeForLayoutPass +{ + ASDN::MutexLocker l(__instanceLock__); + return [self _locked_constrainedSizeForLayoutPass]; +} + - (ASSizeRange)_locked_constrainedSizeForLayoutPass { // TODO: The logic in -_u_setNeedsLayoutFromAbove seems correct and doesn't use this method. // logic seems correct. For what case does -this method need to do the CGSizeEqual checks? // IF WE CAN REMOVE BOUNDS CHECKS HERE, THEN WE CAN ALSO REMOVE "REQUESTED FROM ABOVE" CHECK - + + ASAssertLocked(__instanceLock__); + CGSize boundsSizeForLayout = ASCeilSizeValues(self.threadSafeBounds.size); - + // Checkout if constrained size of pending or calculated display node layout can be used if (_pendingDisplayNodeLayout != nullptr && (_pendingDisplayNodeLayout->requestedLayoutFromAbove - || CGSizeEqualToSize(_pendingDisplayNodeLayout->layout.size, boundsSizeForLayout))) { + || CGSizeEqualToSize(_pendingDisplayNodeLayout->layout.size, boundsSizeForLayout))) { // We assume the size from the last returned layoutThatFits: layout was applied so use the pending display node // layout constrained size return _pendingDisplayNodeLayout->constrainedSize; @@ -444,7 +461,7 @@ - (ASSizeRange)_locked_constrainedSizeForLayoutPass - (void)_layoutSublayouts { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASLayout *layout; { @@ -503,6 +520,7 @@ - (BOOL)_isLayoutTransitionInvalid - (BOOL)_locked_isLayoutTransitionInvalid { + ASAssertLocked(__instanceLock__); if (ASHierarchyStateIncludesLayoutPending(_hierarchyState)) { ASLayoutElementContext *context = ASLayoutElementGetCurrentContext(); if (context == nil || _pendingTransitionID != context.transitionID) { @@ -535,18 +553,10 @@ - (void)transitionLayoutWithAnimation:(BOOL)animated measurementCompletion:(void(^)())completion { ASDisplayNodeAssertMainThread(); - - ASSizeRange sizeRange; - { - ASDN::MutexLocker l(__instanceLock__); - sizeRange = [self _locked_constrainedSizeForLayoutPass]; - } - - [self transitionLayoutWithSizeRange:sizeRange + [self transitionLayoutWithSizeRange:[self _constrainedSizeForLayoutPass] animated:animated shouldMeasureAsync:shouldMeasureAsync measurementCompletion:completion]; - } - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize @@ -653,9 +663,9 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize // Update calculated layout let previousLayout = _calculatedDisplayNodeLayout; let pendingLayout = std::make_shared(newLayout, - constrainedSize, - constrainedSize.max, - newLayoutVersion); + constrainedSize, + constrainedSize.max, + newLayoutVersion); [self _locked_setCalculatedDisplayNodeLayout:pendingLayout]; // Setup pending layout transition for animation @@ -865,8 +875,8 @@ - (void)didCompleteLayoutTransition:(id)context */ - (void)_completePendingLayoutTransition { - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock); - + ASAssertUnlocked(__instanceLock__); + ASLayoutTransition *pendingLayoutTransition = nil; { ASDN::MutexLocker l(__instanceLock__); @@ -875,7 +885,7 @@ - (void)_completePendingLayoutTransition [self _locked_setCalculatedDisplayNodeLayout:pendingLayoutTransition.pendingLayout]; } } - + if (pendingLayoutTransition != nil) { [self _completeLayoutTransition:pendingLayoutTransition]; [self _pendingLayoutTransitionDidComplete]; @@ -895,6 +905,8 @@ - (void)_completeLayoutTransition:(ASLayoutTransition *)layoutTransition // Trampoline to the main thread if necessary if (ASDisplayNodeThreadIsMain() || layoutTransition.isSynchronous == NO) { + // Committing the layout transition will result in subnode insertions and removals, both of which must be called without the lock held + ASAssertUnlocked(__instanceLock__); [layoutTransition commitTransition]; } else { // Subnode insertions and removals need to happen always on the main thread if at least one subnode is already loaded @@ -948,12 +960,13 @@ - (void)_assertSubnodeState - (void)_pendingLayoutTransitionDidComplete { // This assertion introduces a breaking behavior for nodes that has ASM enabled but also manually manage some subnodes. - // Let's gate it behind YOGA flag and remove it right after a branch cut. + // Let's gate it behind YOGA flag. #if YOGA [self _assertSubnodeState]; #endif // Subclass hook + ASAssertUnlocked(__instanceLock__); [self calculatedLayoutDidChange]; // Grab lock after calling out to subclass @@ -998,6 +1011,7 @@ - (void)_setCalculatedDisplayNodeLayout:(std::shared_ptr)di - (void)_locked_setCalculatedDisplayNodeLayout:(std::shared_ptr)displayNodeLayout { + ASAssertLocked(__instanceLock__); ASDisplayNodeAssertTrue(displayNodeLayout->layout.layoutElement == self); ASDisplayNodeAssertTrue(displayNodeLayout->layout.size.width >= 0.0); ASDisplayNodeAssertTrue(displayNodeLayout->layout.size.height >= 0.0); diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 765039937..848b9a4b8 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -460,11 +460,14 @@ - (void)dealloc - (BOOL)_locked_shouldLoadViewOrLayer { + ASAssertLocked(__instanceLock__); return !_flags.isDeallocating && !(_hierarchyState & ASHierarchyStateRasterized); } - (UIView *)_locked_viewToLoad { + ASAssertLocked(__instanceLock__); + UIView *view = nil; if (_viewBlock) { view = _viewBlock(); @@ -503,6 +506,7 @@ - (UIView *)_locked_viewToLoad - (CALayer *)_locked_layerToLoad { + ASAssertLocked(__instanceLock__); ASDisplayNodeAssert(_flags.layerBacked, @"_layerToLoad is only for layer-backed nodes"); CALayer *layer = nil; @@ -521,6 +525,8 @@ - (CALayer *)_locked_layerToLoad - (void)_locked_loadViewOrLayer { + ASAssertLocked(__instanceLock__); + if (_flags.layerBacked) { TIME_SCOPED(_debugTimeToCreateView); _layer = [self _locked_layerToLoad]; @@ -549,7 +555,7 @@ - (void)_locked_loadViewOrLayer - (void)_didLoad { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeLogEvent(self, @"didLoad"); as_log_verbose(ASNodeLog(), "didLoad %@", self); TIME_SCOPED(_debugTimeForDidLoad); @@ -584,7 +590,7 @@ - (BOOL)isNodeLoaded if (ASDisplayNodeThreadIsMain()) { // Because the view and layer can only be created and destroyed on Main, that is also the only thread // where the state of this property can change. As an optimization, we can avoid locking. - return [self _locked_isNodeLoaded]; + return _loaded(self); } else { ASDN::MutexLocker l(__instanceLock__); return [self _locked_isNodeLoaded]; @@ -593,7 +599,8 @@ - (BOOL)isNodeLoaded - (BOOL)_locked_isNodeLoaded { - return (_view != nil || (_layer != nil && _flags.layerBacked)); + ASAssertLocked(__instanceLock__); + return _loaded(self); } #pragma mark - Misc Setter / Getter @@ -696,6 +703,7 @@ - (_ASDisplayLayer *)asyncLayer - (_ASDisplayLayer *)_locked_asyncLayer { + ASAssertLocked(__instanceLock__); return [_layer isKindOfClass:[_ASDisplayLayer class]] ? (_ASDisplayLayer *)_layer : nil; } @@ -754,6 +762,7 @@ - (CGRect)threadSafeBounds - (CGRect)_locked_threadSafeBounds { + ASAssertLocked(__instanceLock__); return _threadSafeBounds; } @@ -1014,7 +1023,7 @@ - (void)invalidateCalculatedLayout - (void)__layout { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); BOOL loaded = NO; { @@ -1066,7 +1075,7 @@ - (void)layoutDidFinish { // Hook for subclasses ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertTrue(self.isNodeLoaded); } @@ -1226,6 +1235,7 @@ - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize - (id)_locked_layoutElementThatFits:(ASSizeRange)constrainedSize { + ASAssertLocked(__instanceLock__); __ASDisplayNodeCheckForLayoutMethodOverrides; BOOL measureLayoutSpec = _measurementOptions & ASDisplayNodePerformanceMeasurementOptionLayoutSpec; @@ -1256,7 +1266,7 @@ - (void)layout { // Hook for subclasses ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertTrue(self.isNodeLoaded); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(nodeDidLayout)]) { @@ -1310,6 +1320,7 @@ - (BOOL)displaysAsynchronously */ - (BOOL)_locked_displaysAsynchronously { + ASAssertLocked(__instanceLock__); return checkFlag(Synchronous) == NO && _flags.displaysAsynchronously; } @@ -2093,7 +2104,7 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) { - (ASDisplayNode *)supernode { #if CHECK_LOCKING_SAFETY - if (__instanceLock__.ownedByCurrentThread()) { + if (__instanceLock__.locked()) { NSLog(@"WARNING: Accessing supernode while holding recursive instance lock of this node is worrisome. It's likely that you will soon try to acquire the supernode's lock, and this can easily cause deadlocks."); } #endif @@ -2187,7 +2198,7 @@ - (NSArray *)subnodes /* * Central private helper method that should eventually be called if submethods add, insert or replace subnodes - * This method is called with thread affinity. + * This method is called with thread affinity and without lock held. * * @param subnode The subnode to insert * @param subnodeIndex The index in _subnodes to insert it @@ -2197,7 +2208,9 @@ - (NSArray *)subnodes */ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnodeIndex sublayerIndex:(NSInteger)sublayerIndex andRemoveSubnode:(ASDisplayNode *)oldSubnode { - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASDisplayNodeAssertThreadAffinity(self); + ASAssertUnlocked(__instanceLock__); + as_log_verbose(ASNodeLog(), "Insert subnode %@ at index %zd of %@ and remove subnode %@", subnode, subnodeIndex, self, oldSubnode); if (subnode == nil || subnode == self) { @@ -2405,6 +2418,7 @@ - (void)insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)bel - (void)_insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)below { ASDisplayNodeAssertThreadAffinity(self); + ASAssertUnlocked(__instanceLock__); if (subnode == nil) { ASDisplayNodeFailAssert(@"Cannot insert a nil subnode"); @@ -2468,6 +2482,7 @@ - (void)insertSubnode:(ASDisplayNode *)subnode aboveSubnode:(ASDisplayNode *)abo - (void)_insertSubnode:(ASDisplayNode *)subnode aboveSubnode:(ASDisplayNode *)above { ASDisplayNodeAssertThreadAffinity(self); + ASAssertUnlocked(__instanceLock__); if (subnode == nil) { ASDisplayNodeFailAssert(@"Cannot insert a nil subnode"); @@ -2529,6 +2544,7 @@ - (void)insertSubnode:(ASDisplayNode *)subnode atIndex:(NSInteger)idx - (void)_insertSubnode:(ASDisplayNode *)subnode atIndex:(NSInteger)idx { ASDisplayNodeAssertThreadAffinity(self); + ASAssertUnlocked(__instanceLock__); if (subnode == nil) { ASDisplayNodeFailAssert(@"Cannot insert a nil subnode"); @@ -2565,7 +2581,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atIndex:(NSInteger)idx - (void)_removeSubnode:(ASDisplayNode *)subnode { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); // Don't call self.supernode here because that will retain/autorelease the supernode. This method -_removeSupernode: is often called while tearing down a node hierarchy, and the supernode in question might be in the middle of its -dealloc. The supernode is never messaged, only compared by value, so this is safe. // The particular issue that triggers this edge case is when a node calls -removeFromSupernode on a subnode from within its own -dealloc method. @@ -2591,7 +2607,7 @@ - (void)removeFromSupernode - (void)_removeFromSupernode { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); __instanceLock__.lock(); __weak ASDisplayNode *supernode = _supernode; @@ -2605,7 +2621,7 @@ - (void)_removeFromSupernode - (void)_removeFromSupernodeIfEqualTo:(ASDisplayNode *)supernode { ASDisplayNodeAssertThreadAffinity(self); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); __instanceLock__.lock(); @@ -2698,6 +2714,7 @@ - (void)__decrementVisibilityNotificationsDisabled - (void)_locked_layoutPlaceholderIfNecessary { + ASAssertLocked(__instanceLock__); if ([self _locked_shouldHavePlaceholderLayer]) { [self _locked_setupPlaceholderLayerIfNeeded]; } @@ -2707,12 +2724,14 @@ - (void)_locked_layoutPlaceholderIfNecessary - (BOOL)_locked_shouldHavePlaceholderLayer { + ASAssertLocked(__instanceLock__); return (_placeholderEnabled && [self _implementsDisplay]); } - (void)_locked_setupPlaceholderLayerIfNeeded { ASDisplayNodeAssertMainThread(); + ASAssertLocked(__instanceLock__); if (!_placeholderLayer) { _placeholderLayer = [CALayer layer]; @@ -2760,7 +2779,7 @@ - (void)__enterHierarchy { ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"Should not cause recursive __enterHierarchy"); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeLogEvent(self, @"enterHierarchy"); // Profiling has shown that locking this method is beneficial, so each of the property accesses don't have to lock and unlock. @@ -2809,7 +2828,7 @@ - (void)__exitHierarchy { ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(!_flags.isExitingHierarchy, @"Should not cause recursive __exitHierarchy"); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeLogEvent(self, @"exitHierarchy"); // Profiling has shown that locking this method is beneficial, so each of the property accesses don't have to lock and unlock. @@ -2916,7 +2935,7 @@ - (void)willEnterHierarchy ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(_flags.isEnteringHierarchy, @"You should never call -willEnterHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isExitingHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); if (![self supportsRangeManagedInterfaceState]) { self.interfaceState = ASInterfaceStateInHierarchy; @@ -2928,7 +2947,7 @@ - (void)didEnterHierarchy { ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"You should never call -didEnterHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isExitingHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); ASDisplayNodeAssert(_flags.isInHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); } - (void)didExitHierarchy @@ -2936,7 +2955,7 @@ - (void)didExitHierarchy ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(_flags.isExitingHierarchy, @"You should never call -didExitHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); // This case is important when tearing down hierarchies. We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for // things like data analytics about user content viewing. We cannot call the method in the dealloc as any incidental retain operations in client code would fail. @@ -3058,7 +3077,7 @@ - (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState ASDisplayNodeAssertMainThread(); // This method manages __instanceLock__ itself, to ensure the lock is not held while didEnter/Exit(.*)State methods are called, thus avoid potential deadlocks - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASInterfaceState oldState = ASInterfaceStateNone; ASInterfaceState newState = ASInterfaceStateNone; @@ -3185,7 +3204,7 @@ - (void)prepareForCATransactionCommit - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfaceState)oldState { // Subclass hook - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertMainThread(); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { @@ -3229,7 +3248,7 @@ - (void)didEnterVisibleState { // subclass override ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(didEnterVisibleState)]) { [delegate didEnterVisibleState]; @@ -3244,7 +3263,7 @@ - (void)didExitVisibleState { // subclass override ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(didExitVisibleState)]) { [delegate didExitVisibleState]; @@ -3262,7 +3281,7 @@ - (void)didEnterDisplayState { // subclass override ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(didEnterDisplayState)]) { [delegate didEnterDisplayState]; @@ -3274,7 +3293,7 @@ - (void)didExitDisplayState { // subclass override ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(didExitDisplayState)]) { [delegate didExitDisplayState]; @@ -3316,7 +3335,7 @@ - (void)recursivelyClearPreloadedData - (void)didEnterPreloadState { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); // If this node has ASM enabled and is not yet visible, force a layout pass to apply its applicable pending layout, if any, // so that its subnodes are inserted/deleted and start preloading right away. @@ -3340,7 +3359,7 @@ - (void)didEnterPreloadState - (void)didExitPreloadState { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); for (id delegate in _interfaceStateDelegates) { if ([delegate respondsToSelector:@selector(didExitPreloadState)]) { [delegate didExitPreloadState]; @@ -3445,6 +3464,7 @@ - (BOOL)pointInside:(CGPoint)point withEvent:(UIEvent *)event - (void)_locked_applyPendingStateToViewOrLayer { ASDisplayNodeAssertMainThread(); + ASAssertLocked(__instanceLock__); ASDisplayNodeAssert(self.nodeLoaded, @"must have a view or layer"); TIME_SCOPED(_debugTimeToApplyPendingState); @@ -3464,7 +3484,7 @@ - (void)_locked_applyPendingStateToViewOrLayer - (void)applyPendingViewState { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); + ASAssertUnlocked(__instanceLock__); ASDN::MutexLocker l(__instanceLock__); // FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout @@ -3483,6 +3503,7 @@ - (void)applyPendingViewState - (void)_locked_applyPendingViewState { ASDisplayNodeAssertMainThread(); + ASAssertLocked(__instanceLock__); ASDisplayNodeAssert([self _locked_isNodeLoaded], @"Expected node to be loaded before applying pending state."); if (_flags.layerBacked) { diff --git a/Source/ASImageNode+AnimatedImage.mm b/Source/ASImageNode+AnimatedImage.mm index febbb725e..bd085eabe 100644 --- a/Source/ASImageNode+AnimatedImage.mm +++ b/Source/ASImageNode+AnimatedImage.mm @@ -21,6 +21,7 @@ #import #import #import +#import #import #import #import @@ -50,6 +51,8 @@ - (void)setAnimatedImage:(id )animatedImage - (void)_locked_setAnimatedImage:(id )animatedImage { + ASAssertLocked(__instanceLock__); + if (ASObjectIsEqual(_animatedImage, animatedImage) && (animatedImage == nil || animatedImage.playbackReady)) { return; } @@ -124,6 +127,8 @@ - (void)setCoverImageCompleted:(UIImage *)coverImage - (void)_locked_setCoverImageCompleted:(UIImage *)coverImage { + ASAssertLocked(__instanceLock__); + _displayLinkLock.lock(); BOOL setCoverImage = (_displayLink == nil) || _displayLink.paused; _displayLinkLock.unlock(); @@ -141,6 +146,8 @@ - (void)setCoverImage:(UIImage *)coverImage - (void)_locked_setCoverImage:(UIImage *)coverImage { + ASAssertLocked(__instanceLock__); + //If we're a network image node, we want to set the default image so //that it will correctly be restored if it exits the range. #if ASAnimatedImageDebug @@ -182,6 +189,8 @@ - (void)setShouldAnimate:(BOOL)shouldAnimate - (void)_locked_setShouldAnimate:(BOOL)shouldAnimate { + ASAssertLocked(__instanceLock__); + // This test is explicitly done and not ASPerformBlockOnMainThread as this would perform the block immediately // on main if called on main thread and we have to call methods locked or unlocked based on which thread we are on if (ASDisplayNodeThreadIsMain()) { @@ -215,6 +224,8 @@ - (void)startAnimating - (void)_locked_startAnimating { + ASAssertLocked(__instanceLock__); + // It should be safe to call self.interfaceState in this case as it will only grab the lock of the superclass if (!ASInterfaceStateIncludesVisible(self.interfaceState)) { return; @@ -258,6 +269,7 @@ - (void)stopAnimating - (void)_locked_stopAnimating { ASDisplayNodeAssertMainThread(); + ASAssertLocked(__instanceLock__); #if ASAnimatedImageDebug NSLog(@"stopping animation: %p", self); diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm index b572c9d97..d060d8993 100644 --- a/Source/ASImageNode.mm +++ b/Source/ASImageNode.mm @@ -244,6 +244,7 @@ - (void)setImage:(UIImage *)image - (void)_locked_setImage:(UIImage *)image { + ASAssertLocked(__instanceLock__); if (ASObjectIsEqual(_image, image)) { return; } diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm index be5bf1df4..cace5498e 100755 --- a/Source/ASNetworkImageNode.mm +++ b/Source/ASNetworkImageNode.mm @@ -20,6 +20,7 @@ #import #import #import +#import #import #import #import @@ -149,6 +150,8 @@ - (void)setImage:(UIImage *)image - (void)_locked_setImage:(UIImage *)image { + ASAssertLocked(__instanceLock__); + BOOL imageWasSetExternally = (image != nil); BOOL shouldCancelAndClear = imageWasSetExternally && (imageWasSetExternally != _imageWasSetExternally); _imageWasSetExternally = imageWasSetExternally; @@ -175,6 +178,7 @@ - (void)_setImage:(UIImage *)image - (void)_locked__setImage:(UIImage *)image { + ASAssertLocked(__instanceLock__); [super _locked_setImage:image]; } @@ -508,6 +512,8 @@ - (void)_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResume - (void)_locked_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResume { + ASAssertLocked(__instanceLock__); + [self _locked_cancelImageDownloadWithResumePossibility:storeResume]; [self _locked_setAnimatedImage:nil]; @@ -532,6 +538,8 @@ - (void)_cancelImageDownloadWithResumePossibility:(BOOL)storeResume - (void)_locked_cancelImageDownloadWithResumePossibility:(BOOL)storeResume { + ASAssertLocked(__instanceLock__); + if (!_downloadIdentifier) { return; } diff --git a/Source/ASTextNode.mm b/Source/ASTextNode.mm index 75507a994..3ffcfe569 100644 --- a/Source/ASTextNode.mm +++ b/Source/ASTextNode.mm @@ -26,9 +26,10 @@ #import #import #import +#import +#import #import #import -#import #import #import @@ -343,17 +344,20 @@ - (ASTextKitRenderer *)_rendererWithBounds:(CGRect)bounds - (ASTextKitRenderer *)_locked_renderer { + ASAssertLocked(__instanceLock__); return [self _locked_rendererWithBounds:[self _locked_threadSafeBounds]]; } - (ASTextKitRenderer *)_locked_rendererWithBounds:(CGRect)bounds { + ASAssertLocked(__instanceLock__); bounds = UIEdgeInsetsInsetRect(bounds, _textContainerInset); return rendererForAttributes([self _locked_rendererAttributes], bounds.size); } - (ASTextKitAttributes)_locked_rendererAttributes { + ASAssertLocked(__instanceLock__); return { .attributedString = _attributedText, .truncationAttributedString = [self _locked_composedTruncationText], @@ -1272,6 +1276,7 @@ - (NSRange)_additionalTruncationMessageRangeWithVisibleRange:(NSRange)visibleRan */ - (NSAttributedString *)_locked_composedTruncationText { + ASAssertLocked(__instanceLock__); if (_composedTruncationText == nil) { if (_truncationAttributedText != nil && _additionalTruncationMessage != nil) { NSMutableAttributedString *newComposedTruncationString = [[NSMutableAttributedString alloc] initWithAttributedString:_truncationAttributedText]; @@ -1297,6 +1302,7 @@ - (NSAttributedString *)_locked_composedTruncationText */ - (NSAttributedString *)_locked_prepareTruncationStringForDrawing:(NSAttributedString *)truncationString { + ASAssertLocked(__instanceLock__); truncationString = ASCleanseAttributedStringOfCoreTextAttributes(truncationString); NSMutableAttributedString *truncationMutableString = [truncationString mutableCopy]; // Grab the attributes from the full string diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index 92dc243b2..7d3d8d7b3 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -19,8 +19,9 @@ #import #import #import -#import #import +#import +#import #import #import @@ -1098,7 +1099,7 @@ - (NSRange)_additionalTruncationMessageRangeWithVisibleRange:(NSRange)visibleRan */ - (NSAttributedString *)_locked_composedTruncationText { - ASDisplayNodeAssertLockOwnedByCurrentThread(__instanceLock__); + ASAssertLocked(__instanceLock__); if (_composedTruncationText == nil) { if (_truncationAttributedText != nil && _additionalTruncationMessage != nil) { NSMutableAttributedString *newComposedTruncationString = [[NSMutableAttributedString alloc] initWithAttributedString:_truncationAttributedText]; @@ -1124,7 +1125,7 @@ - (NSAttributedString *)_locked_composedTruncationText */ - (NSAttributedString *)_locked_prepareTruncationStringForDrawing:(NSAttributedString *)truncationString { - ASDisplayNodeAssertLockOwnedByCurrentThread(__instanceLock__); + ASAssertLocked(__instanceLock__); NSMutableAttributedString *truncationMutableString = [truncationString mutableCopy]; // Grab the attributes from the full string if (_attributedText.length > 0) { diff --git a/Source/ASVideoNode.mm b/Source/ASVideoNode.mm index d67c72ab0..3b8c9bd37 100644 --- a/Source/ASVideoNode.mm +++ b/Source/ASVideoNode.mm @@ -338,6 +338,7 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N if (self.playerState != ASVideoNodePlayerStatePlaying) { self.playerState = ASVideoNodePlayerStateReadyToPlay; if (_shouldBePlaying && ASInterfaceStateIncludesVisible(self.interfaceState)) { + ASUnlockScope(self); [self play]; } } @@ -360,6 +361,8 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N if (self.playerState == ASVideoNodePlayerStateLoading && _delegateFlags.delegateVideoNodeDidRecoverFromStall) { [self.delegate videoNodeDidRecoverFromStall:self]; } + + ASUnlockScope(self); [self play]; // autoresume after buffer catches up } } else if ([keyPath isEqualToString:kplaybackBufferEmpty]) { diff --git a/Source/ASVideoPlayerNode.mm b/Source/ASVideoPlayerNode.mm index 7859448a6..c864c10a9 100644 --- a/Source/ASVideoPlayerNode.mm +++ b/Source/ASVideoPlayerNode.mm @@ -26,6 +26,7 @@ #import #import #import +#import #import static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext; @@ -334,6 +335,8 @@ - (void)cleanCachedControls - (void)_locked_createPlaybackButton { + ASAssertLocked(__instanceLock__); + if (_playbackButtonNode == nil) { _playbackButtonNode = [[ASDefaultPlaybackButton alloc] init]; _playbackButtonNode.style.preferredSize = CGSizeMake(16.0, 22.0); @@ -357,6 +360,8 @@ - (void)_locked_createPlaybackButton - (void)_locked_createFullScreenButton { + ASAssertLocked(__instanceLock__); + if (_fullScreenButtonNode == nil) { _fullScreenButtonNode = [[ASButtonNode alloc] init]; _fullScreenButtonNode.style.preferredSize = CGSizeMake(16.0, 22.0); @@ -374,6 +379,8 @@ - (void)_locked_createFullScreenButton - (void)_locked_createElapsedTextField { + ASAssertLocked(__instanceLock__); + if (_elapsedTextNode == nil) { _elapsedTextNode = [[ASTextNode alloc] init]; _elapsedTextNode.attributedText = [self timeLabelAttributedStringForString:@"00:00" @@ -387,6 +394,8 @@ - (void)_locked_createElapsedTextField - (void)_locked_createDurationTextField { + ASAssertLocked(__instanceLock__); + if (_durationTextNode == nil) { _durationTextNode = [[ASTextNode alloc] init]; _durationTextNode.attributedText = [self timeLabelAttributedStringForString:@"00:00" @@ -401,6 +410,8 @@ - (void)_locked_createDurationTextField - (void)_locked_createScrubber { + ASAssertLocked(__instanceLock__); + if (_scrubberNode == nil) { __weak __typeof__(self) weakSelf = self; _scrubberNode = [[ASDisplayNode alloc] initWithViewBlock:^UIView * _Nonnull { @@ -445,6 +456,8 @@ - (void)_locked_createScrubber - (void)_locked_createControlFlexGrowSpacer { + ASAssertLocked(__instanceLock__); + if (_controlFlexGrowSpacerSpec == nil) { _controlFlexGrowSpacerSpec = [[ASStackLayoutSpec alloc] init]; _controlFlexGrowSpacerSpec.style.flexGrow = 1.0; diff --git a/Source/Details/ASThread.h b/Source/Details/ASThread.h index 5c61aa783..362f96c5b 100644 --- a/Source/Details/ASThread.h +++ b/Source/Details/ASThread.h @@ -108,13 +108,13 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) * Enable this flag to collect information on the owning thread and ownership level of a mutex. * These properties are useful to determine if a mutext has been acquired and in case of a recursive mutex, how many times that happened. * - * This flag also enable locking assertions (e.g ASDisplayNodeAssertLockUnownedByCurrentThread(node)). + * This flag also enable locking assertions (e.g ASAssertUnlocked(node)). * The assertions are useful when you want to indicate and enforce the locking policy/expectation of methods. * To determine when and which methods acquired a (recursive) mutex (to debug deadlocks, for example), * put breakpoints at some assertions. When the breakpoints hit, walk through stack trace frames * and check ownership count of the mutex. */ -#define CHECK_LOCKING_SAFETY 0 +#define CHECK_LOCKING_SAFETY 1 #if TIME_LOCKER #import @@ -138,11 +138,11 @@ ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id __strong *lockPtr) * and check ownership count of the mutex. */ #if CHECK_LOCKING_SAFETY -#define ASDisplayNodeAssertLockUnownedByCurrentThread(lock) ASDisplayNodeAssertFalse(lock.ownedByCurrentThread()) -#define ASDisplayNodeAssertLockOwnedByCurrentThread(lock) ASDisplayNodeAssert(lock.ownedByCurrentThread()) +#define ASAssertUnlocked(lock) ASDisplayNodeAssertFalse(lock.locked()) +#define ASAssertLocked(lock) ASDisplayNodeAssert(lock.locked(), @"Lock must be held by current thread") #else -#define ASDisplayNodeAssertLockUnownedByCurrentThread(lock) -#define ASDisplayNodeAssertLockOwnedByCurrentThread(lock) +#define ASAssertUnlocked(lock) +#define ASAssertLocked(lock) #endif namespace ASDN { @@ -346,7 +346,7 @@ namespace ASDN { pthread_mutex_t *mutex () { return &_m; } #if CHECK_LOCKING_SAFETY - bool ownedByCurrentThread() { + bool locked() { return _count > 0 && pthread_mach_thread_np(pthread_self()) == _owner; } #endif diff --git a/Source/Private/ASDisplayNode+FrameworkPrivate.h b/Source/Private/ASDisplayNode+FrameworkPrivate.h index 245d9f382..95a1e8dc3 100644 --- a/Source/Private/ASDisplayNode+FrameworkPrivate.h +++ b/Source/Private/ASDisplayNode+FrameworkPrivate.h @@ -297,6 +297,11 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyStateChange(ASHierarc */ - (BOOL)_isLayoutTransitionInvalid; +/** + * Same as @c -_isLayoutTransitionInvalid but must be called with the node's instance lock held. + */ +- (BOOL)_locked_isLayoutTransitionInvalid; + /** * Internal method that can be overriden by subclasses to add specific behavior after the measurement of a layout * transition did finish. diff --git a/Source/Private/ASDisplayNode+UIViewBridge.mm b/Source/Private/ASDisplayNode+UIViewBridge.mm index 939ed642c..55d970467 100644 --- a/Source/Private/ASDisplayNode+UIViewBridge.mm +++ b/Source/Private/ASDisplayNode+UIViewBridge.mm @@ -41,8 +41,6 @@ #define DISPLAYNODE_USE_LOCKS 1 -#define __loaded(node) (node->_view != nil || (node->_layer != nil && node->_flags.layerBacked)) - #if DISPLAYNODE_USE_LOCKS #define _bridge_prologue_read ASDN::MutexLocker l(__instanceLock__); ASDisplayNodeAssertThreadAffinity(self) #define _bridge_prologue_write ASDN::MutexLocker l(__instanceLock__) @@ -59,7 +57,7 @@ /// returns NO. Otherwise, the pending state can be scheduled and flushed *before* you get a chance /// to apply it. ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNode *node) { - BOOL loaded = __loaded(node); + BOOL loaded = _loaded(node); if (ASDisplayNodeThreadIsMain()) { return loaded; } else { @@ -70,7 +68,7 @@ ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNo } }; -#define _getFromViewOrLayer(layerProperty, viewAndPendingViewStateProperty) __loaded(self) ? \ +#define _getFromViewOrLayer(layerProperty, viewAndPendingViewStateProperty) _loaded(self) ? \ (_view ? _view.viewAndPendingViewStateProperty : _layer.layerProperty )\ : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty @@ -80,9 +78,9 @@ ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNo #define _setToViewOnly(viewAndPendingViewStateProperty, viewAndPendingViewStateExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \ if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); } else { ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); } -#define _getFromViewOnly(viewAndPendingViewStateProperty) __loaded(self) ? _view.viewAndPendingViewStateProperty : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty +#define _getFromViewOnly(viewAndPendingViewStateProperty) _loaded(self) ? _view.viewAndPendingViewStateProperty : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty -#define _getFromLayer(layerProperty) __loaded(self) ? _layer.layerProperty : ASDisplayNodeGetPendingState(self).layerProperty +#define _getFromLayer(layerProperty) _loaded(self) ? _layer.layerProperty : ASDisplayNodeGetPendingState(self).layerProperty #define _setToLayer(layerProperty, layerValueExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNodeGetPendingState(self).layerProperty = (layerValueExpr); } @@ -304,7 +302,7 @@ - (void)setFrame:(CGRect)rect struct ASDisplayNodeFlags flags = _flags; BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), flags.layerBacked); - BOOL nodeLoaded = __loaded(self); + BOOL nodeLoaded = _loaded(self); BOOL isMainThread = ASDisplayNodeThreadIsMain(); if (!specialPropertiesHandling) { BOOL canReadProperties = isMainThread || !nodeLoaded; @@ -416,7 +414,7 @@ - (void)setNeedsLayout { _bridge_prologue_write; shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); - loaded = __loaded(self); + loaded = _loaded(self); viewOrLayer = _view ?: _layer; if (shouldApply == NO && loaded) { // The node is loaded but we're not on main. @@ -447,7 +445,7 @@ - (void)layoutIfNeeded { _bridge_prologue_write; shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); - loaded = __loaded(self); + loaded = _loaded(self); viewOrLayer = _view ?: _layer; if (shouldApply == NO && loaded) { // The node is loaded but we're not on main. @@ -658,7 +656,7 @@ - (void)setAutoresizingMask:(UIViewAutoresizing)mask - (UIViewContentMode)contentMode { _bridge_prologue_read; - if (__loaded(self)) { + if (_loaded(self)) { if (_flags.layerBacked) { return ASDisplayNodeUIContentModeFromCAContentsGravity(_layer.contentsGravity); } else { @@ -909,7 +907,7 @@ - (UIEdgeInsets)safeAreaInsets _bridge_prologue_read; if (AS_AVAILABLE_IOS(11.0)) { - if (!_flags.layerBacked && __loaded(self)) { + if (!_flags.layerBacked && _loaded(self)) { return self.view.safeAreaInsets; } } @@ -938,7 +936,7 @@ - (void)setInsetsLayoutMarginsFromSafeArea:(BOOL)insetsLayoutMarginsFromSafeArea } } - shouldNotifyAboutUpdate = __loaded(self) && (!AS_AT_LEAST_IOS11 || _flags.layerBacked); + shouldNotifyAboutUpdate = _loaded(self) && (!AS_AT_LEAST_IOS11 || _flags.layerBacked); } if (shouldNotifyAboutUpdate) { @@ -975,6 +973,7 @@ - (void)setLayerCornerRadius:(CGFloat)newLayerCornerRadius - (BOOL)_locked_insetsLayoutMarginsFromSafeArea { + ASAssertLocked(__instanceLock__); if (AS_AVAILABLE_IOS(11.0)) { if (!_flags.layerBacked) { return _getFromViewOnly(insetsLayoutMarginsFromSafeArea); @@ -996,7 +995,7 @@ - (BOOL)_locked_insetsLayoutMarginsFromSafeArea // - In case the node is loaded // - Check if the node has a view and get the value from the view if loaded or from the pending state // - If view is not available, e.g. the node is layer backed return the property value -#define _getAccessibilityFromViewOrProperty(nodeProperty, viewAndPendingViewStateProperty) __loaded(self) ? \ +#define _getAccessibilityFromViewOrProperty(nodeProperty, viewAndPendingViewStateProperty) _loaded(self) ? \ (_view ? _view.viewAndPendingViewStateProperty : nodeProperty )\ : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h index 252202d51..4c668a3f3 100644 --- a/Source/Private/ASDisplayNodeInternal.h +++ b/Source/Private/ASDisplayNodeInternal.h @@ -65,6 +65,9 @@ typedef NS_OPTIONS(uint_least32_t, ASDisplayNodeAtomicFlags) YogaLayoutInProgress = 1 << 1, }; +// Can be called without the node's lock. Client is responsible for thread safety. +#define _loaded(node) (node->_layer != nil) + #define checkFlag(flag) ((_atomicFlags.load() & flag) != 0) // Returns the old value of the flag as a BOOL. #define setFlag(flag, x) (((x ? _atomicFlags.fetch_or(flag) \