Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix calling CALayer out of the main thread #1762

Merged
merged 1 commit into from
Feb 11, 2020
Merged

fix calling CALayer out of the main thread #1762

merged 1 commit into from
Feb 11, 2020

Conversation

vovasty
Copy link
Contributor

@vovasty vovasty commented Jan 9, 2020 •
edited
Loading

Background:
In some cases asyncTraitCollectionDidChangeWithPreviousTraitCollection might be called on loaded node in background. In this case we access layer from background thread. It locks main thread and may lead to dead lock.

Fixes:

  1. Ensure that asyncTraitCollectionDidChangeWithPreviousTraitCollection accesses layer from the main thread.
  2. Resolve background color using current trait collection.
  3. Simplify locking.

e.g.

Thread 0:
0	libsystem_kernel.dylib	
___psynch_mutexwait
1	libsystem_pthread.dylib	
__pthread_mutex_firstfit_lock_wait
2	libsystem_pthread.dylib	
__pthread_mutex_firstfit_lock_slow
3	libc++.1.dylib	
std::__1::recursive_mutex::lock()
4	Pinterest	
ASThread.h:177:22
std::__1::lock_guard<AS::Mutex>::lock_guard(AS::Mutex&)
5	Pinterest	
ASDisplayNode.mm:760:15
-[ASDisplayNode isLayerBacked]
6	Pinterest	
ASDisplayNode.mm:1761:12
-[ASDisplayNode actionForLayer:forKey:]
7	Pinterest	
_ASDisplayView.mm:89:29
-[_ASDisplayView actionForLayer:forKey:]
8	QuartzCore	
-[CALayer actionForKey:]
9	QuartzCore	
CA::Layer::begin_change(CA::Transaction*, unsigned int, objc_object*, objc_object*&)
10	QuartzCore	
CA::Layer::setter(unsigned int, _CAValueType, void const*)
11	QuartzCore	
-[CALayer setAffineTransform:]
12	UIKitCore	
-[UIView setTransform:]



Thread 11:
0	libsystem_kernel.dylib	
___ulock_wait
1	libsystem_platform.dylib	
__os_unfair_lock_lock_slow
2	QuartzCore	
CA::Layer::getter(unsigned int, _CAValueType, void*)
3	QuartzCore	
-[CALayer backgroundColor]
4	Pinterest	
ASDisplayNode.mm:451:41
-[ASDisplayNode asyncTraitCollectionDidChangeWithPreviousTraitCollection:]
5	Pinterest	
ASDisplayNode+Layout.mm:141:7
-[ASDisplayNode(ASLayoutElement) setPrimitiveTraitCollection:]
6	Pinterest	
ASTraitCollection.mm:20:13
ASTraitCollectionPropagateDown
7	Pinterest	
ASTraitCollection.mm:24:5
ASTraitCollectionPropagateDown
8	Pinterest	
ASTraitCollection.mm:24:5
ASTraitCollectionPropagateDown
9	Pinterest	
ASTraitCollection.mm:24:5
ASTraitCollectionPropagateDown
10	Pinterest	
ASDisplayNode+LayoutSpec.mm:82:5
-[ASDisplayNode(ASLayoutSpec) calculateLayoutLayoutSpec:]
11	Pinterest	
ASDisplayNode.mm:1085:14
-[ASDisplayNode calculateLayoutThatFits:]
12	Pinterest	
ASDisplayNode.mm:1067:22
-[ASDisplayNode calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
13	Pinterest	
ASDisplayNode+Layout.mm:109:14
-[ASDisplayNode(ASLayoutElement) layoutThatFits:parentSize:]
14	Pinterest	
ASStackUnpositionedLayout.mm:65:22
crossChildLayout
15	Pinterest	
ASStackUnpositionedLayout.mm:689:21
___ZL43layoutItemsAlongUnconstrainedStackDimensionRNSt3__16vectorI21ASStackLayoutSpecItemNS_9allocatorIS1_EEEERK22ASStackLayoutSpecStylebRK11ASSizeRange6CGSizeb_block_invoke
16	Pinterest	
ASStackUnpositionedLayout.mm:84:7
dispatchApplyIfNeeded
17	Pinterest	
ASStackUnpositionedLayout.mm:684:3
layoutItemsAlongUnconstrainedStackDimension
18	Pinterest	
ASStackLayoutSpec.mm:145:35
-[ASStackLayoutSpec calculateLayoutThatFits:]
19	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
20	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
21	Pinterest	
ASInsetLayoutSpec.mm:101:25
-[ASInsetLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
22	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
23	Pinterest	
ASDisplayNode+LayoutSpec.mm:93:5
-[ASDisplayNode(ASLayoutSpec) calculateLayoutLayoutSpec:]
24	Pinterest	
ASDisplayNode.mm:1085:14
-[ASDisplayNode calculateLayoutThatFits:]
25	Pinterest	
ASDisplayNode.mm:1067:22
-[ASDisplayNode calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
26	Pinterest	
ASDisplayNode+Layout.mm:109:14
-[ASDisplayNode(ASLayoutElement) layoutThatFits:parentSize:]
27	Pinterest	
ASStackUnpositionedLayout.mm:65:22
crossChildLayout
28	Pinterest	
ASStackUnpositionedLayout.mm:689:21
___ZL43layoutItemsAlongUnconstrainedStackDimensionRNSt3__16vectorI21ASStackLayoutSpecItemNS_9allocatorIS1_EEEERK22ASStackLayoutSpecStylebRK11ASSizeRange6CGSizeb_block_invoke
29	Pinterest	
ASStackUnpositionedLayout.mm:84:7
dispatchApplyIfNeeded
30	Pinterest	
ASStackUnpositionedLayout.mm:684:3
layoutItemsAlongUnconstrainedStackDimension
31	Pinterest	
ASStackLayoutSpec.mm:145:35
-[ASStackLayoutSpec calculateLayoutThatFits:]
32	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
33	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
34	Pinterest	
ASOverlayLayoutSpec.mm:77:31
-[ASOverlayLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
35	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
36	Pinterest	
ASOverlayLayoutSpec.mm:71:30
-[ASOverlayLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
37	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
38	Pinterest	
ASStackUnpositionedLayout.mm:65:22
crossChildLayout
39	Pinterest	
ASStackUnpositionedLayout.mm:689:21
___ZL43layoutItemsAlongUnconstrainedStackDimensionRNSt3__16vectorI21ASStackLayoutSpecItemNS_9allocatorIS1_EEEERK22ASStackLayoutSpecStylebRK11ASSizeRange6CGSizeb_block_invoke
40	Pinterest	
ASStackUnpositionedLayout.mm:84:7
dispatchApplyIfNeeded
41	Pinterest	
ASStackUnpositionedLayout.mm:684:3
layoutItemsAlongUnconstrainedStackDimension
42	Pinterest	
ASStackLayoutSpec.mm:145:35
-[ASStackLayoutSpec calculateLayoutThatFits:]
43	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
44	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
45	Pinterest	
ASDisplayNode+LayoutSpec.mm:93:5
-[ASDisplayNode(ASLayoutSpec) calculateLayoutLayoutSpec:]
46	Pinterest	
ASDisplayNode.mm:1085:14
-[ASDisplayNode calculateLayoutThatFits:]
47	Pinterest	
ASDisplayNode.mm:1067:22
-[ASDisplayNode calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
48	Pinterest	
ASDisplayNode+Layout.mm:109:14
-[ASDisplayNode(ASLayoutElement) layoutThatFits:parentSize:]
49	Pinterest	
ASInsetLayoutSpec.mm:101:25
-[ASInsetLayoutSpec calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
50	Pinterest	
ASLayoutSpec.mm:74:1
-[ASLayoutSpec layoutThatFits:parentSize:]
51	Pinterest	
ASDisplayNode+LayoutSpec.mm:93:5
-[ASDisplayNode(ASLayoutSpec) calculateLayoutLayoutSpec:]
52	Pinterest	
ASDisplayNode.mm:1085:14
-[ASDisplayNode calculateLayoutThatFits:]
53	Pinterest	
ASDisplayNode.mm:1067:22
-[ASDisplayNode calculateLayoutThatFits:restrictedToSize:relativeToParentSize:]
54	Pinterest	
ASDisplayNode+Layout.mm:109:14
-[ASDisplayNode(ASLayoutElement) layoutThatFits:parentSize:]
55	Pinterest	
ASDataController.mm:191:16
-[ASDataController _layoutNode:withConstrainedSize:]
56	Pinterest	
ASDataController.mm:171:9
__47-[ASDataController _allocateNodesFromElements:]_block_invoke
57	libdispatch.dylib	
__dispatch_client_callout2
58	libdispatch.dylib	
__dispatch_apply_invoke_and_wait
59	libdispatch.dylib	
_dispatch_apply_f
60	Pinterest	
ASDispatch.mm:25:7
ASDispatchApply
61	Pinterest	
ASDataController.mm:154:5
-[ASDataController _allocateNodesFromElements:]
62	Pinterest	
ASDataController.mm:645:7
__40-[ASDataController updateWithChangeSet:]_block_invoke_2.196
63	libdispatch.dylib	
__dispatch_call_block_and_release
64	libdispatch.dylib	
__dispatch_client_callout
65	libdispatch.dylib	
__dispatch_lane_serial_drain
66	libdispatch.dylib	
__dispatch_lane_invoke
67	libdispatch.dylib	
__dispatch_workloop_worker_thread
68	libsystem_pthread.dylib	
__pthread_wqthread

@vovasty vovasty requested review from bolsinga and nguyenhuy and removed request for rahul-malik January 24, 2020 19:28
nguyenhuy
nguyenhuy requested changes Jan 28, 2020
View reviewed changes
Source/ASDisplayNode.mm Show resolved Hide resolved
@vovasty vovasty force-pushed the vlad/asyncTraitCollectionDidChangeWithPreviousTraitCollection branch 2 times, most recently from f8d3d1d to 5ea9e41 Compare February 10, 2020 20:45
nguyenhuy
nguyenhuy reviewed Feb 10, 2020
View reviewed changes
Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right to me. Let's address the last nit and merge it!

Source/ASDisplayNode.mm Outdated Show resolved Hide resolved
@vovasty vovasty force-pushed the vlad/asyncTraitCollectionDidChangeWithPreviousTraitCollection branch from 5ea9e41 to 42d8e72 Compare February 10, 2020 22:43
@nguyenhuy nguyenhuy merged commit 3674402 into TextureGroup:master Feb 11, 2020
vovasty added a commit that referenced this pull request Feb 20, 2020
@vovasty
Revert "Fix calling CALayer out of the main thread (#1762)"
2b86883 
This reverts commit 3674402.
@vovasty vovasty mentioned this pull request Feb 20, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nguyenhuy nguyenhuy nguyenhuy approved these changes

@bolsinga bolsinga Awaiting requested review from bolsinga

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vovasty @nguyenhuy