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

[ASDisplayNode] Locking on isSynchronous is avoidable and sometimes deadlocks. #64

Closed
appleguy opened this issue Apr 23, 2017 · 3 comments
Closed

[ASDisplayNode] Locking on isSynchronous is avoidable and sometimes deadlocks. #64

appleguy opened this issue Apr 23, 2017 · 3 comments

Comments

@appleguy
Copy link
Member

appleguy commented Apr 23, 2017 •
edited
Loading

I attempted converting it to a const flag, but there are a couple places this doesn't work - mainly the setViewBlock: method (can we get rid of those / why do they exist?)

It would work for all use cases to convert it to a std::atomic.

Unless we make a change, there does exist a scenario where we can deadlock due to hitting isSynchronous.

(lldb) bt all
* thread #1: tid = 0x31f6, 0x0000000180b03f6c libsystem_kernel.dylib`__psynch_mutexwait + 8, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
    frame #0: 0x0000000180b03f6c libsystem_kernel.dylib`__psynch_mutexwait + 8
    frame #1: 0x0000000180bd239c libsystem_pthread.dylib`_pthread_mutex_lock_wait + 96
    frame #2: 0x0000000180bd257c libsystem_pthread.dylib`_pthread_mutex_lock_slow + 296
  * frame #3: 0x00000001015d5d48 App`ASDN::Mutex::lock(this=0x000000015925c458) + 60 at ASThread.h:198
    frame #4: 0x00000001015d5ce0 App`ASDN::Locker<ASDN::Mutex>::Locker(this=0x000000016fddf4b0, l=0x000000015925c458) + 68 at ASThread.h:84
    frame #5: 0x00000001015d2d1c App`ASDN::Locker<ASDN::Mutex>::Locker(this=0x000000016fddf4b0, l=0x000000015925c458) + 64 at ASThread.h:83
    frame #6: 0x0000000101611f48 App`::-[ASDisplayNode isSynchronous](self=0x000000015925c450, _cmd="isSynchronous") + 80 at ASDisplayNode.mm:753
    frame #7: 0x0000000101627428 App`shouldDisableNotificationsForMovingBetweenParents(from=0x000000015925c450, to=0x0000000157d2df10) + 148 at ASDisplayNode.mm:2453
    frame #8: 0x000000010162716c App`::-[ASDisplayNode _insertSubnode:atSubnodeIndex:sublayerIndex:andRemoveSubnode:](self=0x0000000157d2df10, _cmd="_insertSubnode:atSubnodeIndex:sublayerIndex:andRemoveSubnode:", subnode=0x00000001591b9fa0, subnodeIndex=1, sublayerIndex=1, oldSubnode=0x0000000000000000) + 2276 at ASDisplayNode.mm:2636
    frame #9: 0x000000010162bcc4 App`::-[ASDisplayNode _insertSubnode:atIndex:](self=0x0000000157d2df10, _cmd="_insertSubnode:atIndex:", subnode=0x00000001591b9fa0, idx=1) + 1992 at ASDisplayNode.mm:2931
    frame #10: 0x00000001017759d0 App`::-[ASLayoutTransition applySubnodeInsertions](self=0x00000001592a84a0, _cmd="applySubnodeInsertions") + 840 at ASLayoutTransition.mm:112
    frame #11: 0x0000000101775648 App`::-[ASLayoutTransition commitTransition](self=0x00000001592a84a0, _cmd="commitTransition") + 68 at ASLayoutTransition.mm:95
    frame #12: 0x000000010161e5a0 App`::-[ASDisplayNode _completeLayoutTransition:](self=0x0000000157d2df10, _cmd="_completeLayoutTransition:", layoutTransition=0x00000001592a84a0) + 216 at ASDisplayNode.mm:1767
    frame #13: 0x000000010161e434 App`::-[ASDisplayNode _completePendingLayoutTransition](self=0x0000000157d2df10, _cmd="_completePendingLayoutTransition") + 276 at ASDisplayNode.mm:1749
    frame #14: 0x0000000101614e08 App`::-[ASDisplayNode _locked_measureNodeWithBoundsIfNecessary:](self=0x0000000157d2df10, _cmd="_locked_measureNodeWithBoundsIfNecessary:", bounds=(origin = (x = 0, y = 0), size = (width = 768, height = 40))) + 2540 at ASDisplayNode.mm:1045
    frame #15: 0x0000000101614238 App`::-[ASDisplayNode __layout](self=0x0000000157d2df10, _cmd="__layout") + 660 at ASDisplayNode.mm:960
    frame #16: 0x000000010170fb48 App`::-[_ASDisplayLayer layoutSublayers](self=0x0000000157e2bbf0, _cmd="layoutSublayers") + 440 at _ASDisplayLayer.mm:132
    frame #17: 0x0000000183a495d0 QuartzCore`CA::Layer::layout_if_needed(CA::Transaction*) + 292
    frame #18: 0x0000000183a49490 QuartzCore`CA::Layer::layout_and_display_if_needed(CA::Transaction*) + 32
    frame #19: 0x0000000183a48ac0 QuartzCore`CA::Context::commit_transaction(CA::Transaction*) + 252
    frame #20: 0x0000000183a48820 QuartzCore`CA::Transaction::commit() + 500
    frame #21: 0x0000000183a9c190 QuartzCore`CA::Display::DisplayLink::dispatch_items(unsigned long long, unsigned long long, unsigned long long) + 592
    frame #22: 0x00000001811e1e54 IOKit`IODispatchCalloutFromCFMessage + 372
    frame #23: 0x0000000180f09030 CoreFoundation`__CFMachPortPerform + 180
    frame #24: 0x0000000180f217d4 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 56
    frame #25: 0x0000000180f20f0c CoreFoundation`__CFRunLoopDoSource1 + 436
    frame #26: 0x0000000180f1ec64 CoreFoundation`__CFRunLoopRun + 1800
    frame #27: 0x0000000180e48c50 CoreFoundation`CFRunLoopRunSpecific + 384
    frame #28: 0x0000000182730088 GraphicsServices`GSEventRunModal + 180
    frame #29: 0x000000018612a088 UIKit`UIApplicationMain + 204
    frame #30: 0x000000010008d198 App`main(argc=1, argv=0x000000016fde3a00) + 236 at main.m:41
    frame #31: 0x00000001809e68b8 libdyld.dylib`start + 4

  thread #27: tid = 0x3300, 0x0000000180b03f6c libsystem_kernel.dylib`__psynch_mutexwait + 8, queue = 'com.apple.root.default-qos'
    frame #0: 0x0000000180b03f6c libsystem_kernel.dylib`__psynch_mutexwait + 8
    frame #1: 0x0000000180bd239c libsystem_pthread.dylib`_pthread_mutex_lock_wait + 96
    frame #2: 0x0000000180bd257c libsystem_pthread.dylib`_pthread_mutex_lock_slow + 296
    frame #3: 0x00000001015d5d48 App`ASDN::Mutex::lock(this=0x0000000157d2df18) + 60 at ASThread.h:198
    frame #4: 0x00000001015d5ce0 App`ASDN::Locker<ASDN::Mutex>::Locker(this=0x000000016eb05830, l=0x0000000157d2df18) + 68 at ASThread.h:84
    frame #5: 0x00000001015d2d1c App`ASDN::Locker<ASDN::Mutex>::Locker(this=0x000000016eb05830, l=0x0000000157d2df18) + 64 at ASThread.h:83
    frame #6: 0x00000001016267bc App`::-[ASDisplayNode subnodes](self=0x0000000157d2df10, _cmd="subnodes") + 68 at ASDisplayNode.mm:2558
    frame #7: 0x0000000101612ca0 App`::-[ASDisplayNode sublayoutElements](self=0x0000000157d2df10, _cmd="sublayoutElements") + 68 at ASDisplayNode.mm:844
    frame #8: 0x00000001017336bc App`::ASLayoutElementPerformBlockOnEveryElement(element=0x0000000157d2df10, block=0x00000001016fa700)(id)) + 208 at ASLayoutElement.mm:32
    frame #9: 0x0000000101733780 App`::ASLayoutElementPerformBlockOnEveryElement(element=0x0000000157d2b8a0, block=0x00000001016fa700)(id)) + 404 at ASLayoutElement.mm:33
    frame #10: 0x00000001016fa69c App`::ASTraitCollectionPropagateDown(root=0x0000000157d2b8a0, traitCollection=<unavailable>) + 172 at ASTraitCollection.m:23
    frame #11: 0x00000001016163c8 App`::-[ASDisplayNode calculateLayoutThatFits:](self=0x000000015925c450, _cmd="calculateLayoutThatFits:", constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308))) + 2992 at ASDisplayNode.mm:1152
    frame #12: 0x0000000101615670 App`::-[ASDisplayNode calculateLayoutThatFits:restrictedToSize:relativeToParentSize:](self=0x000000015925c450, _cmd="calculateLayoutThatFits:restrictedToSize:relativeToParentSize:", constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308)), parentSize=(width = 768, height = 1.7976931348623157E+308)) + 428 at ASDisplayNode.mm:1092
    frame #13: 0x0000000101613910 App`::-[ASDisplayNode layoutThatFits:parentSize:](self=0x000000015925c450, _cmd="layoutThatFits:parentSize:", constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308)), parentSize=(width = 768, height = 1.7976931348623157E+308)) + 1724 at ASDisplayNode.mm:901
    frame #14: 0x00000001016375a0 App`::-[ASDisplayNode measureWithSizeRange:](self=0x000000015925c450, _cmd="measureWithSizeRange:", constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308))) + 136 at ASDisplayNode.mm:3990
    frame #15: 0x0000000101613224 App`::-[ASDisplayNode layoutThatFits:](self=0x000000015925c450, _cmd="layoutThatFits:", constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308))) + 120 at ASDisplayNode.mm:876
    frame #16: 0x00000001016db260 App`::-[ASDataController _layoutNode:withConstrainedSize:](self=0x00000001513c6200, _cmd="_layoutNode:withConstrainedSize:", node=0x000000015925c450, constrainedSize=(min = (width = 768, height = 0), max = (width = 768, height = 1.7976931348623157E+308))) + 708 at ASDataController.mm:175
    frame #17: 0x00000001016dc060 App`::__45-[ASDataController _layoutNodesFromContexts:]_block_invoke((null)=<unavailable>, i=84) + 972 at ASDataController.mm:205
    frame #18: 0x00000001016e4d64 App`::___ZL15ASDispatchApplymPU28objcproto17OS_dispatch_queue8NSObjectmU13block_pointerFvmE_block_invoke((null)=<unavailable>) + 140 at ASDispatch.h:27
    frame #19: 0x0000000105c61a7c libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #20: 0x0000000105c61a3c libdispatch.dylib`_dispatch_client_callout + 16
    frame #21: 0x0000000105c70c9c libdispatch.dylib`_dispatch_root_queue_drain + 2344
    frame #22: 0x0000000105c70364 libdispatch.dylib`_dispatch_worker_thread3 + 132
    frame #23: 0x0000000180bcd470 libsystem_pthread.dylib`_pthread_wqthread + 1092
    frame #24: 0x0000000180bcd020 libsystem_pthread.dylib`start_wqthread + 4

  thread #28: tid = 0x331b, 0x0000000180ae9014 libsystem_kernel.dylib`semaphore_wait_trap + 8, queue = 'org.AsyncDisplayKit.ASDataController.editingTransactionQueue:0x1513c6200'
    frame #0: 0x0000000180ae9014 libsystem_kernel.dylib`semaphore_wait_trap + 8
    frame #1: 0x0000000105c73efc libdispatch.dylib`_dispatch_group_wait_slow + 260
    frame #2: 0x00000001016dbbdc App`ASDispatchApply(iterationCount=92, queue=0x0000000105ca9140, threadCount=4, work=0x00000001016dbc94) block_pointer) + 492 at ASDispatch.h:31
    frame #3: 0x00000001016db7ac App`::-[ASDataController _layoutNodesFromContexts:](self=0x00000001513c6200, _cmd="_layoutNodesFromContexts:", elements=92 elements) + 916 at ASDataController.mm:191
    frame #4: 0x00000001016dae50 App`::-[ASDataController batchLayoutNodesFromContexts:batchSize:batchCompletion:](self=0x00000001513c6200, _cmd="batchLayoutNodesFromContexts:batchSize:batchCompletion:", elements=92 elements, batchSize=92, batchCompletionHandler=0x00000001016e00d4) + 1092 at ASDataController.mm:160
    frame #5: 0x00000001016dffec App`::__40-[ASDataController updateWithChangeSet:]_block_invoke.193((null)=<unavailable>) + 360 at ASDataController.mm:490
    frame #6: 0x0000000105c61a7c libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #7: 0x0000000105c61a3c libdispatch.dylib`_dispatch_client_callout + 16
    frame #8: 0x0000000105c6e554 libdispatch.dylib`_dispatch_queue_drain + 1036
    frame #9: 0x0000000105c6572c libdispatch.dylib`_dispatch_queue_invoke + 464
    frame #10: 0x0000000105c61a3c libdispatch.dylib`_dispatch_client_callout + 16
    frame #11: 0x0000000105c70c9c libdispatch.dylib`_dispatch_root_queue_drain + 2344
    frame #12: 0x0000000105c70364 libdispatch.dylib`_dispatch_worker_thread3 + 132
    frame #13: 0x0000000180bcd470 libsystem_pthread.dylib`_pthread_wqthread + 1092
    frame #14: 0x0000000180bcd020 libsystem_pthread.dylib`start_wqthread + 4
@3a4oT
Copy link

3a4oT commented Apr 24, 2017

wondering does it related to #66

@Adlai-Holler
Copy link
Member

We have to keep -setViewBlock: for the time being, or else subclasses cannot set view blocks for their superclass, since -initWithViewBlock: is a convenience initializer and cannot be a designated initializer (or else all node classes would have to provide implementations of it which is awful. The alternative is to make something like ASViewWrapperNode and ASLayerWrapperNode classes that provide those as designated initializers.

In any case, yeah let's use Obj-C or c11 atomics. Obj-C is slightly faster but we should prefer whichever gives us the cleanest code. @appleguy

@appleguy appleguy mentioned this issue Apr 30, 2017
Closed
@appleguy
Copy link
Member Author

Fixed in #89

@CoderXL CoderXL mentioned this issue Sep 28, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@appleguy @Adlai-Holler @3a4oT