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

memory leak while unmounting the lottie component #1010

Closed
pSapien opened this issue Apr 16, 2023 · 16 comments · Fixed by #1055
Closed

memory leak while unmounting the lottie component #1010

pSapien opened this issue Apr 16, 2023 · 16 comments · Fixed by #1055
Assignees

Comments

@pSapien
Copy link

pSapien commented Apr 16, 2023

Description

The Lottie React Native component seems to be causing a memory leak when unmounted. After the component is unmounted, the device does not release the memory, which causes the RAM usage to spike up.
Haven't checked in Android devices yet.

Steps to Reproduce

  1. Mount a list of Lottie components inside a modal-like component
  2. Unmount the component continuously
  3. Watch the RAM level spike up
  • Code
import {useState, useRef, useEffect} from 'react';
import {View, Text, StatusBar} from 'react-native';
import LottieView from 'lottie-react-native';

StatusBar.setHidden(true);

const lottieImage = require('./assets/money_mouth_face.json');
const RE_RENDER_LIMIT = 50;
const INTERVAL_LIMIT = 1500;
const LOTTIE_VIEW_COUNT = 12;

export default function App() {
  const [show, setShow] = useState(false);
  const renderCountRef = useRef(0);

  useEffect(() => {
    const lastIntervalRef = setInterval(() => {
      if (renderCountRef.current >= RE_RENDER_LIMIT) {
        setShow(false);
        clearInterval(lastIntervalRef);
      } else {
        renderCountRef.current++;
        setShow(p => !p);
      }
    }, INTERVAL_LIMIT);
  }, []);

  if (renderCountRef.current >= RE_RENDER_LIMIT) return <Text>Done...</Text>;

  return (
    <View style={{flex: 1, alignItems: 'center', justifyContent: 'center'}}>
      {show ? (
        Array.from({length: LOTTIE_VIEW_COUNT}).map((p, idx) => {
          return (
            <LottieView
              source={lottieImage}
              style={{height: 40, width: 40}}
              key={idx}
              autoPlay
              renderMode="HARDWARE"
            />
          );
        })
      ) : (
        <Text>Re-rendering...</Text>
      )}
    </View>
  );
}

Repo: https://github.com/pSapien/lottie-memory-leak

Screen.Recording.2023-04-16.at.21.30.50.mov

Expected behavior:

  • When the Lottie React Native component is unmounted, I expect the device to release the memory used by the component, which should prevent the app's RAM usage from spiking up.

Actual behavior:

When the Lottie React Native component is unmounted, the device does not release the memory used by the component, causing the memory leak.

Versions

lottie-react-native: 6.0.0-rc.3, 6.0.0-rc.2, 5.1.5

@pSapien pSapien changed the title memory leak while unmounting the lottie component inside a list memory leak while unmounting the lottie component Apr 16, 2023
@nikitph
Copy link

nikitph commented Apr 17, 2023

This is a very accurate representation of things. We have been using animationref.current. Reset() on unmount but this has to be automatic process.

@pSapien
Copy link
Author

pSapien commented Apr 17, 2023

This is a very accurate representation of things. We have been using animationref.current. Reset() on unmount but this has to be an automatic process.

Well, I went ahead and wrote this in the LottieView library component itself

  componentWillUnmount(): void {
    if (this.props.autoPlay) this.reset();
  }

Although the reset command is being triggered, the problem still persists.

@nikitph
Copy link

nikitph commented Apr 17, 2023 via email

@nikitph
Copy link

nikitph commented Apr 17, 2023 via email

@matinzd matinzd self-assigned this Apr 17, 2023
@matinzd
Copy link
Collaborator

matinzd commented Apr 17, 2023

Thank you for the detailed explanation. I will try to look into that on the weekend.

However, the reset method does not unmount the view. It just resets the frame. @nikitph

Do you have new architecture turned on? Meanwhile, can you try adding removeClippedSubviews on the outer view and see how it behaves? @pSapien

@pSapien
Copy link
Author

pSapien commented Apr 17, 2023

Thank you for the detailed explanation. I will try to look into that on the weekend.

However, the reset method does not unmount the view. It just resets the frame. @nikitph

Do you have new architecture turned on? Meanwhile, can you try adding removeClippedSubviews on the outer view and see how it behaves? @pSapien

Thanks for your quick reply.

In our organizational code, the new architecture is not turned on. I could see no changes by adding removeClippedSubviews though

@nikitph
Copy link

nikitph commented Apr 17, 2023

@matinzd Perhaps. I dont know how or why the above solution works. Just that we get back the RAM engaged by lottie animation this way which we otherwise dont. would love for a more elegant solution if one is possible. 🤘

@nikitph
Copy link

nikitph commented Apr 21, 2023

This is indeed a problem. take a look at the following code and include this component on a screen which refreshes from time to time.

`import React, { forwardRef } from 'react';
import Lottie from 'lottie-react-native';
import { View } from 'react-native';

const NoNotificationAnimation = forwardRef((props, ref) => {
  return (
    <View style={{ width: 200, height: 300 }}>
      <Lottie source={require('../assets/lottie/NotificationNo.json')} autoPlay loop ref={ref} {...props} />
    </View>
  );
});

export default React.memo(NoNotificationAnimation);
`

Lets say the component is used like this

ListEmptyComponent={ isLoading ? ( <View style={styles.loadingContainer}> <View style={styles.loadingSwirl}> <Swirl height={120} white={false} width={100} /> </View> <Text style={{ fontFamily: 'Nexa', fontWeight: 'bold', fontSize: 16, marginTop: 20 }}> Loading Notifications </Text> </View> ) : ( <View style={styles.loadingContainer}> <NoNotificationAnimation ref={noNotificationRef} /> <Text style={{ fontFamily: 'Nexa', fontWeight: 'bold', fontSize: 16, marginTop: 20 }}> No Notifications </Text> </View> ) }

          and the isLoading is being driven like this 
          
          `  const { isLoading, data, fetchNextPage } = useInfiniteQuery('notifications', nonProfitApi.fetchNotifications, {
getNextPageParam: () => {
  return pageNo;
},
refetchInterval: 10000,
onSettled: (datat, error) => {},

});`

Now every time the query fetches, the screen will keep asking for more and more RAM eventually bombing out the App. :(

@bolan9999
Copy link

Any solution for this?

@matinzd
Copy link
Collaborator

matinzd commented May 14, 2023

It appears that Android is not experiencing this issue, as all views are being correctly managed and RAM is released as soon as the view is unmounted. However, I'll investigate further to determine what might be going on behind the scenes with iOS.

lottie_android_perf.mp4

@Nsquik
Copy link

Nsquik commented Jul 4, 2023

Any news @matinzd ?

@matinzd
Copy link
Collaborator

matinzd commented Jul 4, 2023

Any news @matinzd ?

I put some time into that but still no luck. I will try again on the weekend.

@Nsquik
Copy link

Nsquik commented Jul 12, 2023

@matinzd have u tried? 😄

@nikitph
Copy link

nikitph commented Jul 19, 2023

@matinzd would really appreciate an update on this. The library is working near perfect for us except for this problem.

@matinzd
Copy link
Collaborator

matinzd commented Jul 21, 2023

Now, we have addressed the issue and the pull request is in review.

Waiting for @emilioicai with higher access to approve.

emilioicai pushed a commit that referenced this issue Jul 21, 2023
* fix: memory leak on iOS on deallocation

Fixes: #1010

* fix: add weak reference in setSourceURL

* fix: memory leak issue (#1056)

---------

Co-authored-by: Parsa Nasirimehr <40071952+TheRogue76@users.noreply.github.com>
@matinzd matinzd unpinned this issue Jul 21, 2023
@matinzd
Copy link
Collaborator

matinzd commented Jul 22, 2023

The fix is included in v6.0.0-rc.8:

https://github.com/lottie-react-native/lottie-react-native/releases/tag/v6.0.0-rc.8

alyssaharvey3 added a commit to alyssaharvey3/react-native-lottie that referenced this issue Aug 2, 2023
* fix: memory leak on iOS on deallocation

Fixes: lottie-react-native/lottie-react-native#1010

* fix: add weak reference in setSourceURL

* fix: memory leak issue (#1056)

---------

Co-authored-by: Parsa Nasirimehr <40071952+TheRogue76@users.noreply.github.com>
GoldenDragon0710 added a commit to GoldenDragon0710/Lottie-React-Native that referenced this issue Nov 27, 2023
* fix: memory leak on iOS on deallocation

Fixes: lottie-react-native/lottie-react-native#1010

* fix: add weak reference in setSourceURL

* fix: memory leak issue (#1056)

---------

Co-authored-by: Parsa Nasirimehr <40071952+TheRogue76@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants