react-native-reanimated icon indicating copy to clipboard operation
react-native-reanimated copied to clipboard

[ios][bug] Huge issue with LayoutAnimation - Single `Animated.View` with `layout` prop is enough to break un-mounting and de-allocation of any View which has been popped/swiped-back/closed/destroyed via navigation

Open hirbod opened this issue 2 years ago • 47 comments

Description

@piaskowyk @kmagiera

I have found a serious bug with LayoutAnimation. I have been trying to investigate the problem for several days, which is why videos that were in a screen or modal that I closed (goBack/pop/swipe to dismiss) still played even though everything was un-mounted and destroyed. The same is true for cameras (Green Light Indicator is no longer out).

It is sufficient to have a single Reanimated.View / createAnimatedComponent ANYWHERE in the app which uses layout, entering or exiting prop from Reanimated. Once a single View with that prop has been mounted (no matter where), no element that is in a navigation stack will be unmounted anymore, even if the layout hierarchy and lifecycle methods reports that it has been unmounted (eg cleanup function of useEffect). When you remove something conditionally on the screen (eg. with useState), un-mounting works fine. But it looks like a single use of LayoutAnimation is enough to break element un-mounting / de-allocation when closing Navigators, because (this is my guess) LayoutAnimation is still "watching all of them (or snapshotting?)" in order to be able to animate the layout, even when those views are in different screens. Maybe RNS is also an issue here?

I also did some tests, such as setting breakpoints at the deallocate function of expo-av. The methods were never called when layout, entering or exiting were present and mounted anywhere in the app. But if no Reanimated.View with layout was used the breakpoints triggered.

I have also created a video as well as a repro which can be tested with Expo Go.

Expected behavior

Views should correctly un-mount and deallocate once a Navigation screen has been dismissed / popped etc, even when using LayoutAnimation.

Actual behavior & steps to reproduce

To test my findings, simply grab my repro, spin it up, open Tab One and tap on the info icon at the top (headerRight) - as many times as you like. The video will start and stop as soon as the modal has been closed. But if you now switch to Tab two, which mounts a Reanimated.View with layout={Layout}, (imported from REA) and you now simply return to Tab one, you will notice that the video will continue to run as soon as you open and close the modal and it will start multiple videos until the App crashes eventually. If you remove layout from Tab two, you can open and close the modal 1000x and it will work fine. Please note that you need to restart the app when you add or remove layout, fast-refresh is not enough.

I provided a repro and all informations in my issue + in my repro App as Text. Here is also a Video showing the issue (watch with audio please)

Watch with audio

https://user-images.githubusercontent.com/504909/161172649-3d2b600d-41fb-47ec-87bd-89c69f55fe15.mp4

Snack or minimal code example

https://github.com/hirbod/react-reanimatad-layoutanimation-bug

And this single layout prop is enough to trigger the issue (once mounted): https://github.com/hirbod/react-reanimatad-layoutanimation-bug/blob/main/screens/TabTwoScreen.tsx#L7

Package versions

name version
react-native 0.64.3
react-native-reanimated 2.5.0 (but breaks with any version that has LayoutAnimation)
Xcode 13.3
expo SDK 44

Affected platforms

  • [ ] Android
  • [X] iOS
  • [ ] Web

Here is a cross link to RNS, since I am not sure if RNS or REA is the root issue: https://github.com/software-mansion/react-native-screens/issues/1379

hirbod avatar Apr 01 '22 00:04 hirbod

Nice timing; we ran into this bug about 2 hours ago. Like you said any exit/enter/layout prop anywhere breaks anything in a navigation stack everywhere.

adgarcia avatar Apr 01 '22 04:04 adgarcia

Thank you so much for posting this, we have been racking our brains at why all kinds of random things in our app stopped working when we implemented reanimated v2 layout anims (images, svgs, other animations, etc). As soon as we remove the entering/exiting props across the application everything works as normal.

Weirdly enough in our case iOS is not affected but Android is. This is a massive blocker for us, anyway we could get some eyes on this when you guys have some time? @j-piasecki

LeviWilliams avatar Apr 01 '22 19:04 LeviWilliams

I am facing the same issue like when we use entering/exiting/layout prop from reanimated V2 some of the ui views goes invisible. Thanks for investigating this and bringing this up as this is hard to debug and find the root cause. When I commented all entering/exiting/layout props, it works perfectly. A huge blocking for our case as we added many animations and now this adds visibility issue for other UIs. Also, this is for android only in my case In my case somehow when I use any of entering/exiting/layout props, zindex/layering is done wrongly on newly mounted screen , so my invisible ui were behind some fullscreen ui. (I temporarily fixed by reordering component sequence but ideally reanimated should not touch zindex/layering) It would be great if someone can look into this.

RMG0 avatar Apr 01 '22 19:04 RMG0

Glad that my findings helped you. I searched 15 hours without sleep until I found the root cause (I was THIS close to switch RNS to RNN because I was out of ideas. It was a lucky find at the end and never have I ever thought that reanimated could be the cause 😅 )

I did not encounter bugs on Android though, at least in my specific scenarios, everything was working fine. I only did use LayoutAnimation on two FlatLists and a couple of buttons. What hurts the most is my beautiful TextInput animation. I hope I don't have to remove LayoutAnimation and they will be able to fix the issue soon.

I think it would be helpful if you guys could also provide a failing repro on Android, this might help to get both issues resolved.

hirbod avatar Apr 01 '22 20:04 hirbod

@hirbod I'll try to get a minimum repro on Android out asap. We're using exiting/entering in a much larger capacity than just on some lists so we would have to rewrite lots of this with normal react native animations. Would be unfortunate with all the benefits reanimated brings. Also surprised this has not been reported sooner, seems like a severe issue to me.

LeviWilliams avatar Apr 01 '22 21:04 LeviWilliams

I mean, there are alternatives, like https://moti.fyi which use reanimated under the hood without LayoutAnimation (but be careful, they also added support for LayoutAnimation from REA as well if you would like to use it). Its pretty straightforward to animate (also entering/exiting) with AnimatePresence, but there is no comparison to LayoutAnimation (specially how easy it is to use).

The issue is severe imho, I think the feature is just "too new" and not adopted widely enough. Thats why nobody had complaints so far. And I think its pretty hard to find the root cause when things start acting weird (like elements not un-mounting). I would've been curious if I had a LayoutAnimation on a Video, but for me it was a Button inside an invisible (but mounted) tab.

@LeviWilliams feel free to copy my repro to create your case with android.

hirbod avatar Apr 01 '22 21:04 hirbod

I am too facing the same issue on android It crashes everytime on exiting the app and only happens if I use layout animation feature image

pairohit305 avatar Apr 01 '22 21:04 pairohit305

I think thats something different @pairohit305. We should try not to mix up different issues.

hirbod avatar Apr 01 '22 22:04 hirbod

@pairohit305 , this is a different(crash) issue so lets not mix this with thread Thank you

RMG0 avatar Apr 01 '22 22:04 RMG0

@hirbod Hey, I have just started to investigate it. This is a priority issue, I will notify you about progress.

piaskowyk avatar Apr 05 '22 20:04 piaskowyk

@piaskowyk thank you very much! Appreciated.

hirbod avatar Apr 05 '22 20:04 hirbod

I'm also affected by this. I've created a view with a hardcoded height and tried to layout animate only that view using

if ( [targetValues[@"height"] isEqualToNumber:@329] ) {}

in - onViewCreate:() in REAAnimationsManager.

and it works. the unmounted views are deallocated.

In my case, the bug reported by @hirbod is visible when using a combination of AVPlayer and Navigation.reset().

Resetting the stack won't deallocate the avplayer instance, leading to hitting the AVPlayer instances limit which is 15. Due to this no videos will be displayed in the app anymore.

mateioprea avatar Apr 06 '22 15:04 mateioprea

In our case, we were able to resolve some of our issues by reordering the hierarchy. We realized that lots of views using zIndex were affected and what was happening was the views were being rendered in the wrong order causing them to be fully under other root views (????). Still, we observed odd mounting/unmounting behaviour while fixing this and reordering things, certain orders worked and certain didn't.

To be clear these affected views had no relation to any form of layout animations and were even in different navigation stacks. Assuming reanimated was affecting both lifecycle methods and internally modifying zIndex's or something.

LeviWilliams avatar Apr 06 '22 15:04 LeviWilliams

We need to clearly distinguish between all the other errors reported here:

  1. My error is related to iOS - un-mounting and de-allocation of views, most likely there is something in REA and RNS causing this
  2. Similar/related issues reported in this thread for Android (still missing a reproduction for this)
  3. Another report for iOS, with reproduction, most likely related:
    https://github.com/software-mansion/react-native-reanimated/issues/3093 Duplication of Nodes, especially when react-native-svg is used.

All of this might be related or be isolated issues.

hirbod avatar Apr 06 '22 18:04 hirbod

Following up here as we have reproduced what @hirbod initially reported. Our iOS app is affected by an RTMP stream never releasing even when unmounted, similar problem to what is displayed in the reproduction. Commenting out layout props resolves the issue as well.

We can still try to reproduce the zIndex hierarchy issue in a repo if necessary. We've spent a whole pile of time debugging all these issues so reproduction has been a little delayed so far

LeviWilliams avatar Apr 06 '22 21:04 LeviWilliams

Good news, I found the cause of the issue today and I know how to resolve it.

piaskowyk avatar Apr 07 '22 21:04 piaskowyk

This one specific or all of them? :) Anyway, these are great news! Thanks for your work.

hirbod avatar Apr 07 '22 22:04 hirbod

I hope root issue fix would all case :) Thank you for all efforts

RMG0 avatar Apr 07 '22 23:04 RMG0

  • Similar/related issues reported in this thread for Android (still missing a reproduction for this)

@hirbod #2906 provides an android repro for the android issues reported here, but it seems like a separate issue to what is being addressed in PR #3157.

nhanders avatar Apr 12 '22 15:04 nhanders

@piaskowyk there are plenty reports for Android as well. Hopefully we can get all of them sorted :) Thanks for providing a repro, I am sure thats helpful.

hirbod avatar Apr 12 '22 17:04 hirbod

This thread relates to iOS, but it seems to occur on Android as well. I've been working on a very inefficient ScrollView (to be replaced later by something fast), and it exposed this issue quickly. Navigating to a few of these views is enough to hang the app, and removing layout animations from a single, completely unrelated component, fixes this.

I'm not sure how to concretely provide a reproduction case on Android, but I can certainly verify that removing the one and only layout animation we're currently using fixes it.

If there's anything I can do to help, please let me know.

bfricka avatar May 04 '22 01:05 bfricka

LayoutAnimation is undergoing a rewrite, since fixing this issue seems to be something bigger. I think parts of it are cpp, so chances might be that they will fix issues on android as well - but I have no deep knowledge about that. There haven't been any acknowledgments towards android so far, there are also some pretty annoying scrollview issues (android only) going on with reanimated.

hirbod avatar May 04 '22 02:05 hirbod

I experienced same issue on android(react-native-reanimated 2.3.1), after some research, and upgrade to react-native-reanimated 2.8.0 problem is gone. Can provide minimal repro for android if somebody needs it. But from my side looks like problem is resolved in latest versions of react-native-reanimated.

michbil avatar May 06 '22 11:05 michbil

It is not resolved @michbil, LayoutAnimation is currently getting a rewrite

hirbod avatar May 06 '22 11:05 hirbod

@hirbod It resolved my android issue at least, here is reproduction repository. https://github.com/michbil/react-native-reanimated-reproduce-freeze-bug if you upgrade [email protected] here, problem disappears. PS: my repro repository only for android, iOS works fine, so it's possible this is completely different bug.

michbil avatar May 06 '22 11:05 michbil

This issue here is for iOS (which some reports for Android as well)

hirbod avatar May 06 '22 12:05 hirbod

@hirbod Yeah, my comment is mainly for somebody, who has issue on android.

michbil avatar May 06 '22 12:05 michbil

@michbil I'm using 2.8.0 on Android and this issue is not fixed for me. Everything I mentioned in my original reply stands.

bfricka avatar May 07 '22 02:05 bfricka

Using 2.8.0, layout animation is very laggy on Android, while the following error is thrown on iOS.

Exception thrown while executing UI block: RCTScrollView may only contain a single subview, the already set subview looks like: <RCTScrollContentView: 0x7fdb56279760; reactTag: 11115; frame = (0 0; 0 0); layer = <CALayer: 0x6000036ad000>>


__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke
    RCTUIManager.m:1201
__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke.498
__RCTExecuteOnMainQueue_block_invoke
_dispatch_call_block_and_release
_dispatch_client_callout
_dispatch_main_queue_callback_4CF
__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__
__CFRunLoopRun
CFRunLoopRunSpecific
GSEventRunModal
-[UIApplication _run]
UIApplicationMain
main
start_sim
0x0
0x0

lyqht avatar May 13 '22 12:05 lyqht

@hirbod thank you for digging into this. I was so lost when i had this exact issue with a video.

ziyoshams avatar May 23 '22 14:05 ziyoshams