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.

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

Thread should not hold lock #1334

Closed
grangej opened this issue Feb 6, 2019 · 23 comments · Fixed by #1412
Closed

Thread should not hold lock #1334

grangej opened this issue Feb 6, 2019 · 23 comments · Fixed by #1412

Comments

@grangej
Copy link

grangej commented Feb 6, 2019

[Texture Version: p7.4]

Hi I hope this is the right forum for this but I am getting a Assertion after calling view.layoutIfNeeded() inside a UIView animation block.

The function works correctly when the keyboard is displaying but in the case where the keyboard dismisses I am getting the failure.

Code causing the assertion is self.view.layoutIfNeeded()
`
UIView.animate(withDuration: animationDuration, delay: 0, options: animationCurve, animations: { [weak self] in

        guard let self = self else { return }
        
        self.additionalSafeAreaInsets.bottom = intersection.height
        self.view.layoutIfNeeded()
    }, completion: { [weak keyboardAwareVC] (completed) in

        keyboardAwareVC?.onKeyboardFrameDidChange()
    })

`

Log from assertion failure.
2019-02-06 07:14:43.665978-0800 Reach[9726:6494563] *** Assertion failure in void ASDN::Mutex::AssertNotHeld()(), /Users/john/Temp Projects/coClient/Pods/Texture/Source/Details/ASThread.h:204 2019-02-06 07:14:43.680439-0800 Reach[9726:6494563] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Thread should not hold lock' *** First throw call stack: ( 0 CoreFoundation 0x000000011268a1bb __exceptionPreprocess + 331 1 libobjc.A.dylib 0x00000001092af735 objc_exception_throw + 48 2 CoreFoundation 0x0000000112689f42 +[NSException raise:format:arguments:] + 98 3 Foundation 0x000000010cd94940 -[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 166 4 AsyncDisplayKit 0x000000010ae7ab9d _ZN4ASDN5Mutex13AssertNotHeldEv + 589 5 AsyncDisplayKit 0x000000010aeb2092 -[ASDisplayNode __layout] + 514 6 AsyncDisplayKit 0x000000010adba17c -[_ASDisplayLayer layoutSublayers] + 492 7 QuartzCore 0x000000010f78b9d3 _ZN2CA5Layer16layout_if_neededEPNS_11TransactionE + 395 8 UIKitCore 0x000000011b86e077 -[UIView(Hierarchy) layoutBelowIfNeeded] + 1429 9 Reach 0x0000000106acfa3b $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFyycfU_ + 651 10 Reach 0x0000000106acfe95 $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFyycfU_TA + 37 11 Reach 0x000000010663a45d $SIeg_IeyB_TR + 45 12 UIKitCore 0x000000011b876bf2 +[UIView(UIViewAnimationWithBlocks) _setupAnimationWithDuration:delay:view:options:factory:animations:start:animationStateGenerator:completion:] + 558 13 UIKitCore 0x000000011b8770cf +[UIView(UIViewAnimationWithBlocks) animateWithDuration:delay:options:animations:completion:] + 99 14 Reach 0x0000000106acf60b $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VF + 5179 15 Reach 0x0000000106acfbbc $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFTo + 76 16 CoreFoundation 0x00000001125c9bac __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12 17 CoreFoundation 0x00000001125c902f _CFXRegistrationPost + 447 18 CoreFoundation 0x00000001125c8d71 ___CFXNotificationPost_block_invoke + 225 19 CoreFoundation 0x00000001126afae2 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1826 20 CoreFoundation 0x00000001125c8694 _CFXNotificationPost + 964 21 Foundation 0x000000010cdf6589 -[NSNotificationCenter postNotificationName:object:userInfo:] + 66 22 UIKitCore 0x000000011b183016 -[UIInputWindowController postStartNotifications:withInfo:] + 929 23 UIKitCore 0x000000011b185d88 __77-[UIInputWindowController moveFromPlacement:toPlacement:starting:completion:]_block_invoke.1037 + 530 24 UIKitCore 0x000000011b876bf2 +[UIView(UIViewAnimationWithBlocks) _setupAnimationWithDuration:delay:view:options:factory:animations:start:animationStateGenerator:completion:] + 558 25 UIKitCore 0x000000011b877045 +[UIView(UIViewAnimationWithBlocks) _animateWithDuration:delay:options:animations:start:completion:] + 116 26 UIKitCore 0x000000011b1856bc -[UIInputWindowController moveFromPlacement:toPlacement:starting:completion:] + 1695 27 UIKitCore 0x000000011b18dfce -[UIInputWindowController setInputViewSet:] + 1176 28 UIKitCore 0x000000011b184c79 -[UIInputWindowController performOperations:withAnimationStyle:] + 50 29 UIKitCore 0x000000011b20b489 -[UIPeripheralHost(UIKitInternal) setInputViews:animationStyle:] + 1637 30 UIKitCore 0x000000011b2033d6 -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:] + 2352 31 UIKitCore 0x000000011b3c723b -[UIResponder _finishResignFirstResponder] + 267 32 UIKitCore 0x000000011b3c72fd -[UIResponder resignFirstResponder] + 140 33 UIKitCore 0x000000011b6f4dc4 -[UITextView resignFirstResponder] + 93 34 UIKitCore 0x000000011b869c05 -[UIView(Hierarchy) _removeFirstResponderFromSubtree] + 167 35 UIKitCore 0x000000011b86a1b5 __UIViewWillBeRemovedFromSuperview + 72 36 UIKitCore 0x000000011b869fbc -[UIView(Hierarchy) removeFromSuperview] + 95 37 AsyncDisplayKit 0x000000010aec57c1 -[ASDisplayNode _removeFromSupernode:view:layer:] + 177 38 AsyncDisplayKit 0x000000010aec5627 -[ASDisplayNode _removeFromSupernodeIfEqualTo:] + 951 39 AsyncDisplayKit 0x000000010af2166d -[ASLayoutTransition applySubnodeInsertionsAndMoves] + 845 40 AsyncDisplayKit 0x000000010af21318 -[ASLayoutTransition commitTransition] + 72 41 AsyncDisplayKit 0x000000010ae83357 -[ASDisplayNode(ASLayoutTransition) _completeLayoutTransition:] + 215 42 AsyncDisplayKit 0x000000010ae8321f -[ASDisplayNode(ASLayoutTransition) _completePendingLayoutTransition] + 223 43 AsyncDisplayKit 0x000000010ae7c8ec -[ASDisplayNode(ASLayoutInternal) _u_measureNodeWithBoundsIfNecessary:] + 4524 44 AsyncDisplayKit 0x000000010aeb240c -[ASDisplayNode __layout] + 1404 45 AsyncDisplayKit 0x000000010adba17c -[_ASDisplayLayer layoutSublayers] + 492 46 QuartzCore 0x000000010f78b9d3 _ZN2CA5Layer16layout_if_neededEPNS_11TransactionE + 395 47 QuartzCore 0x000000010f7047ca _ZN2CA7Context18commit_transactionEPNS_11TransactionE + 342 48 QuartzCore 0x000000010f73b97e _ZN2CA11Transaction6commitEv + 576 49 QuartzCore 0x000000010f73c6fa _ZN2CA11Transaction17observer_callbackEP19__CFRunLoopObservermPv + 76 50 CoreFoundation 0x00000001125eec27 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23 51 CoreFoundation 0x00000001125e90be __CFRunLoopDoObservers + 430 52 CoreFoundation 0x00000001125e9751 __CFRunLoopRun + 1537 53 CoreFoundation 0x00000001125e8e11 CFRunLoopRunSpecific + 625 54 GraphicsServices 0x0000000115a321dd GSEventRunModal + 62 55 UIKitCore 0x000000011b39981d UIApplicationMain + 140 56 Reach 0x00000001071ba217 main + 71 57 libdyld.dylib 0x000000011449f575 start + 1 ) libc++abi.dylib: terminating with uncaught exception of type NSException (lldb)

@muukii
Copy link
Contributor

muukii commented Feb 12, 2019

I faced the same issue.
But, in my case was the Node-view didn't have a minimum height.
So, I added a minimum height constraint to node.view.

@muukii
Copy link
Contributor

muukii commented Feb 12, 2019

Oops, sorry minimum height constraint did not work.
It seems ASDisplayNode does not support layout by updating height constraint on Animation Block

@muukii
Copy link
Contributor

muukii commented Feb 12, 2019

And it seems the first time layout is important.
If ASDisplayNode has already done layout, ASDisplayNode can layout with animation.

@grangej
Copy link
Author

grangej commented Feb 12, 2019

@muukii I was able tor resolve the issue be reverting to release 2.7, there seems to be changes in the later p releases that causes this.

it seems to be related to UIView animate block

@muukii
Copy link
Contributor

muukii commented Feb 13, 2019

@grangej
wow, thanks for your information.

Do Texture core team know this problem?
cc @nguyenhuy

@grangej
Copy link
Author

grangej commented Feb 13, 2019

I hope so :) hopefully we can get a response , otherwise I may just dig in and create a PR to fix soon.

@grangej
Copy link
Author

grangej commented Feb 15, 2019

@nguyenhuy any idea if this was resolved in 2.8 ?

@alchemist1
Copy link

Having the same problem like @grangej. Reverting back to 2.7 fixed the problem for me as well.

@GeekTree0101
Copy link
Contributor

+1

@GeekTree0101
Copy link
Contributor

2019-02-22 9 48 20

@nivanchikov
Copy link

having the same problem after updating from 2.7 to 2.8. The assert fires on collection node cell sizes evaluation. cc @nguyenhuy

screen shot 2019-03-04 at 09 19 05

@killerham
Copy link

@nlutsenko I'm having the exact same issue. Other than downgrading, have you been able to come up with a solution?

@foxware00
Copy link
Contributor

Also facing the same issue

@grangej
Copy link
Author

grangej commented Mar 13, 2019

@nguyenhuy any thoughts on this? Is this something we are doing wrong, or a bug? Some enlightenment would be greatly appreciated would really like to move to the latest release.

@nguyenhuy
Copy link
Member

nguyenhuy commented Mar 13, 2019

Hey everyone, sorry for the late response!

In 2.8 we added some lock assertions to various methods as an attempt to reduce assumptions on the locking situation when certain methods are called and to surface use cases that can potentially cause locking issues, both within and outside of the framework.

While you all are seeing the same assertion message, from your stack traces, I can say that each of you are actually seeing a different issue.

@grangej Your app hit this assertion during a layout pass which caused a node to remove an old text subnode as part of automatic subnode management, which then caused the subnode's text view to resign from being the first responder which caused the keyboard to be dismissed and triggered your custom animation in which you call -layoutIfNeeded. I believe the lock of the parent/root node was first acquired by the first -__layout call and so later when you called -layoutIfNeeded, the assertion was triggered.

I'd start by asking a few questions:

  • Why do you need to call -layoutIfNeeded within your animation code? Please note that this method is not recommended (by UIKit) to be called directly. Indeed, you'd call -setNeedsLayout and let CA decides when to perform the actual layout. In fact, CA automatically calls -layoutIfNeeded as part of the animation.
  • Perhaps you wanted to call -layoutIfNeeded to trigger a new layout spec calculation? If this is the case, have you tried our layout transition API?
  • Lastly, if you really really need to do what you're doing, you may be able to do it in the next run loop of the main thread -- dispatch to main queue and start an animation there. The reason I used may is that the animation will start after the keyboard started to dismiss which may make the animation looks weird.

For everyone else, please open a new ticket with your own issue. Also, please provide as much information as possible, ideally with a sample project. Issues like this can be hard to reproduce -- ideally our test suite should be able to hit these, and on top of that, we've been using the framework with these new assertions in production settings for a long time and haven't hit any of these issues. Thank you!

Having said that, let me try to give some contexts from the stack traces above:

  • @GeekTree0101 I unfortunately couldn't find any lead from your stack trace. It's a common trace that occurs when a table view needs to layout its cells, updates its ranges (visible, display and preload ranges) and then sets new interface states to some cell nodes. Certain nodes can fall in or out of certain ranges because of this, and they need to react accordingly. If you hit this assertion again, try to print out all the CPU threads and walk backward to figure out the first time that lock was acquired.
  • @nivanchikov I see that you're using Yoga and you hit the assertion very early in the cell node's lifecyle -- while the collection/table view is trying to allocate and layout the node itself for the first time. Have you tried latest master? Please create a new issue with more details and I'll tag people who are familiar with Yoga support to take a look.

@grangej
Copy link
Author

grangej commented Mar 13, 2019

@nguyenhuy I am not sure what you mean by you should not call layoutIfNeeded, this is common in animation blocks? I think you mean layoutSubviews() should not be called directly (which I am not). If you call setNeedsLayout this will not cause the animation to happen along side the keyboard, instead the frame will simply jump on the next layout pass.

I have used layout transition API in other places, but not sure how I can apply it in this case where I need the animation to happen along with keyboard?

I am open for suggestions!

@killerham
Copy link

I’m not using it with keyboard animations. I’m getting this by creating a custom view controller modal transition. In the animation block, I need to call layoutIfNeeded to handle the layout change during the animation.

I’ve also tried performing the animation on the main thread but it eventually got the same crash.

@nguyenhuy
Copy link
Member

nguyenhuy commented Mar 14, 2019

@grangej Oh yes, you're right. I meant -layoutSubviews. Sorry.

After taking another look at your trace, I think this an issue on the framework side. Essentially we hold on to the lock while calling out to -[UIView removeFromSuperview] at the end of a subnode removal. The lock was acquired in -[ASDisplayNode _u_measureNodeWithBoundsIfNecessary:].

Here is the same trace but formatted for readability:

2019-02-06 07:14:43.665978-0800 Reach[9726:6494563] Assertion failure in void ASDN::Mutex::AssertNotHeld()(), /Users/john/Temp Projects/coClient/Pods/Texture/Source/Details/ASThread.h:204
2019-02-06 07:14:43.680439-0800 Reach[9726:6494563] Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Thread should not hold lock' 

First throw call stack: ( 
0 CoreFoundation 0x000000011268a1bb __exceptionPreprocess + 331 

1 libobjc.A.dylib 0x00000001092af735 objc_exception_throw + 48 

2 CoreFoundation 0x0000000112689f42 +[NSException raise:format:arguments:] + 98 

3 Foundation 0x000000010cd94940 -[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 166 

4 AsyncDisplayKit 0x000000010ae7ab9d _ZN4ASDN5Mutex13AssertNotHeldEv + 589 

5 AsyncDisplayKit 0x000000010aeb2092 -[ASDisplayNode __layout] + 514 

6 AsyncDisplayKit 0x000000010adba17c -[_ASDisplayLayer layoutSublayers] + 492 

7 QuartzCore 0x000000010f78b9d3 _ZN2CA5Layer16layout_if_neededEPNS_11TransactionE + 395 

8 UIKitCore 0x000000011b86e077 -[UIView(Hierarchy) layoutBelowIfNeeded] + 1429 

9 Reach 0x0000000106acfa3b $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFyycfU_ + 651 

10 Reach 0x0000000106acfe95 $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFyycfU_TA + 37 

11 Reach 0x000000010663a45d $SIeg_IeyB_TR + 45 

12 UIKitCore 0x000000011b876bf2 +[UIView(UIViewAnimationWithBlocks) _setupAnimationWithDuration:delay:view:options:factory:animations:start:animationStateGenerator:completion:] + 558 

13 UIKitCore 0x000000011b8770cf +[UIView(UIViewAnimationWithBlocks) animateWithDuration:delay:options:animations:completion:] + 99 

14 Reach 0x0000000106acf60b $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VF + 5179 

15 Reach 0x0000000106acfbbc $SSo16UIViewControllerC5ReachE46_onKeyboardFrameWillChangeNotificationReceived33_2EBE1202122E2B2013B55245126FDE4ELLyy10Foundation0I0VFTo + 76 

16 CoreFoundation 0x00000001125c9bac __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12 

17 CoreFoundation 0x00000001125c902f _CFXRegistrationPost + 447 

18 CoreFoundation 0x00000001125c8d71 ___CFXNotificationPost_block_invoke + 225 

19 CoreFoundation 0x00000001126afae2 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1826 

20 CoreFoundation 0x00000001125c8694 _CFXNotificationPost + 964 

21 Foundation 0x000000010cdf6589 -[NSNotificationCenter postNotificationName:object:userInfo:] + 66 

22 UIKitCore 0x000000011b183016 -[UIInputWindowController postStartNotifications:withInfo:] + 929 

23 UIKitCore 0x000000011b185d88 __77-[UIInputWindowController moveFromPlacement:toPlacement:starting:completion:]_block_invoke.1037 + 530 

24 UIKitCore 0x000000011b876bf2 +[UIView(UIViewAnimationWithBlocks) _setupAnimationWithDuration:delay:view:options:factory:animations:start:animationStateGenerator:completion:] + 558 

25 UIKitCore 0x000000011b877045 +[UIView(UIViewAnimationWithBlocks) _animateWithDuration:delay:options:animations:start:completion:] + 116 

26 UIKitCore 0x000000011b1856bc -[UIInputWindowController moveFromPlacement:toPlacement:starting:completion:] + 1695 

27 UIKitCore 0x000000011b18dfce -[UIInputWindowController setInputViewSet:] + 1176 

28 UIKitCore 0x000000011b184c79 -[UIInputWindowController performOperations:withAnimationStyle:] + 50 

29 UIKitCore 0x000000011b20b489 -[UIPeripheralHost(UIKitInternal) setInputViews:animationStyle:] + 1637 

30 UIKitCore 0x000000011b2033d6 -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:] + 2352 

31 UIKitCore 0x000000011b3c723b -[UIResponder _finishResignFirstResponder] + 267 

32 UIKitCore 0x000000011b3c72fd -[UIResponder resignFirstResponder] + 140 

33 UIKitCore 0x000000011b6f4dc4 -[UITextView resignFirstResponder] + 93 

34 UIKitCore 0x000000011b869c05 -[UIView(Hierarchy) _removeFirstResponderFromSubtree] + 167 

35 UIKitCore 0x000000011b86a1b5 __UIViewWillBeRemovedFromSuperview + 72 

***** Calling out to UIView: 36 UIKitCore 0x000000011b869fbc -[UIView(Hierarchy) removeFromSuperview] + 95

37 AsyncDisplayKit 0x000000010aec57c1 -[ASDisplayNode _removeFromSupernode:view:layer:] + 177 

38 AsyncDisplayKit 0x000000010aec5627 -[ASDisplayNode _removeFromSupernodeIfEqualTo:] + 951 

39 AsyncDisplayKit 0x000000010af2166d -[ASLayoutTransition applySubnodeInsertionsAndMoves] + 845 

40 AsyncDisplayKit 0x000000010af21318 -[ASLayoutTransition commitTransition] + 72 

41 AsyncDisplayKit 0x000000010ae83357 -[ASDisplayNode(ASLayoutTransition) _completeLayoutTransition:] + 215 

42 AsyncDisplayKit 0x000000010ae8321f -[ASDisplayNode(ASLayoutTransition) _completePendingLayoutTransition] + 223 

***** Lock acquired and held for the entire method scope: 43 AsyncDisplayKit 0x000000010ae7c8ec -[ASDisplayNode(ASLayoutInternal) _u_measureNodeWithBoundsIfNecessary:] + 4524

44 AsyncDisplayKit 0x000000010aeb240c -[ASDisplayNode __layout] + 1404 

45 AsyncDisplayKit 0x000000010adba17c -[_ASDisplayLayer layoutSublayers] + 492 

46 QuartzCore 0x000000010f78b9d3 _ZN2CA5Layer16layout_if_neededEPNS_11TransactionE + 395 

47 QuartzCore 0x000000010f7047ca _ZN2CA7Context18commit_transactionEPNS_11TransactionE + 342 

48 QuartzCore 0x000000010f73b97e _ZN2CA11Transaction6commitEv + 576 

49 QuartzCore 0x000000010f73c6fa _ZN2CA11Transaction17observer_callbackEP19__CFRunLoopObservermPv + 76 

50 CoreFoundation 0x00000001125eec27 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23 

51 CoreFoundation 0x00000001125e90be __CFRunLoopDoObservers + 430 

52 CoreFoundation 0x00000001125e9751 __CFRunLoopRun + 1537 

53 CoreFoundation 0x00000001125e8e11 CFRunLoopRunSpecific + 625 

54 GraphicsServices 0x0000000115a321dd GSEventRunModal + 62 

55 UIKitCore 0x000000011b39981d UIApplicationMain + 140 

56 Reach 0x00000001071ba217 main + 71 

57 libdyld.dylib 0x000000011449f575 start + 1 ) 
libc++abi.dylib: terminating with uncaught exception of type NSException (lldb) 

@nguyenhuy
Copy link
Member

ok, this is caused by #1204.

@maicki We should find a way to address the root cause and get back control of the lock to make sure subnode insertions and removals are performed lock-free.

@grangej
Copy link
Author

grangej commented Mar 14, 2019

@nguyenhuy Anything more I can do to help here?

@nguyenhuy
Copy link
Member

I’ve just talked to @maicki. I’ll submit a diff to disable the assertion for now while some of us work on different approach(es) for the locking mechanism.

@foxware00
Copy link
Contributor

For everyone else, please open a new ticket with your own issue. Also, please provide as much information as possible, ideally with a sample project. Issues like this can be hard to reproduce -- ideally our test suite should be able to hit these, and on top of that, we've been using the framework with these new assertions in production settings for a long time and haven't hit any of these issues. Thank you!

@nguyenhuy created a separate issue with more information about my specific issue #1404. Many thanks!

@nguyenhuy
Copy link
Member

I've just released 2.8.1 that disables these asserts. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants