facebookarchive / AsyncDisplayKit

Smooth asynchronous user interfaces for iOS apps.
http://asyncdisplaykit.org
Other
13.4k stars 2.2k forks source link

ASDisplayNode crashes #1934

Closed tungvoduc closed 8 years ago

tungvoduc commented 8 years ago

In my app, I got a lot of ASDisplayNode crashes when collectionView reloadData or sometimes even when scrolling ASTableNode. It says "Node should always be marked invisible before deallocating" and is happening quite often. crash

hannahmbanana commented 8 years ago

@tungvoduc: We encountered this issue with a previous version of AsyncDisplayKit, but believe it was fixed on 1.9.81. Could you try master and let us know if you see the same issue?

Separately (but probably not the cause of the crash), it looks like you are using UIKit's Auto Layout. Just so you know, AsyncDisplayKit is not intended to support that.

http://asyncdisplaykit.org/docs/faq.html#asyncdisplaykit-does-not-support-uikit-auto-layout-or-interfacebuilder

ASLayoutSpecs are intended to be an efficient and flexible replacement for UIKit's auto layout, so it should not be necessary to write manual layout code with AsyncDisplayKit.

http://asyncdisplaykit.org/docs/automatic-layout-containers.html

tungvoduc commented 8 years ago

@hannahmbanana : I used AutoLayout in some other parts of the app so those log is not related to the crash. I just tried the latest update and still crashes sometimes.

tungvoduc commented 8 years ago

@hannahmbanana: Okay, now I know what happens. I have a ASTableNode, before getting data from server, I display 10 placeholder rows from a class (let's call it PlaceholderNode). After getting data from server, I reload ASTableNode and everything is fine now except one thing. The 10 instances of PlaceholderNodeCell are not deallocated. They are only deallocated when I reload ASTableNode the second time. So after the second reload, crash will never happen.

In order to make a crash, I have to first select a different tabbarItem in UITabbarController, and then come back, scroll the tableView, this time deinit from PlaceholderNodeCell will be called and make the crash. Any idea why it is happening?

tungvoduc commented 8 years ago

Okay! I made a sample project, the crash has something to do with another dependency (FBShimmeringView). However, everything works fine with ASCollectionNode, only crash in ASTableNode. Check this out: https://github.com/tungvoduc/ASDisplayNodeCrash

You guys can make crash by repeating what I said before:

Adlai-Holler commented 8 years ago

Awesome, thank you! I will look at this today or tomorrow at the latest and hopefully provide a fix.

Adlai-Holler commented 8 years ago

@tungvoduc OK I've found the issue and I've got a fix for you.

When you set FBShimmeringView's contentView, the shimmering view replaces the content view's current superview. AsyncDisplayKit notices this, and detaches the content node from its supernode. Since the FBShimmeringView doesn't have a node, from that point on, the content node is outside of AsyncDisplayKit's node hierarchy, so it stops getting visibility/interface state updates – the node gets frozen in its last known interface state. So if the node was visible when it got removed from the hierarchy, then when it deallocates it goes "how could I be deallocated while I'm visible?!"

The fix I recommend here is to wrap FBShimmeringView in a display node. Here's a modified version of your sample project that does it: ASDisplayNodeCrash-ah-7-20.zip

A new class ShimmeringNode wraps around FBShimmeringView so that the interface state information is always carried down through to the node that got shimmered. Let me know what you think!

tungvoduc commented 8 years ago

@Adlai-Holler : Thank you for the detail. I think this is a very good solution.

Adlai-Holler commented 8 years ago

You're very welcome, thanks for the thorough issue report and I'm glad I could help.