React-native-reanimated: Memory leak when updating stateful value in component

[sparga]

Description

After implementing a swipe gesture on list items inspired by SwipeableListExample.js I noticed a memory leak in my app. I've been able to reproduce with minimal changes to SwipeableListExample.js in Reanimated example project.

It seems that calling a function declared in the same component as the worklet calling it, is creating a memory leak.

Maybe I'm misusing the library but I haven't found any reference to that in the documentation so I'm assuming this is a bug.

Steps To Reproduce

1. Open SwipeableListExample.js in the Example project
2. Declare a stateful value with useState() in the ListItem component.
3. Change the stateful value in the useAnimatedGestureHandler worklet.
4. Take an initial memory snapshot using Safari
5. Run project and navigate multiple times between home and the Swipeable List example
6. Take a second memory snapshot using Safari and look for the number of PanGestureHandler

Expected behavior

Previous PanGestureHandler components and their children should be deallocated when their parent is deallocated or no longer attached to any parent. This is the behavior of the example project without modifications.

Actual behavior

All PanGestureHandler components and their children stay in memory until application is killed.

Screenshots

Safari Dev Tools, list of new PanGestureHandler in memory since initial snapshot.

Detail of one retained PanGestureHandler

Snack or minimal code example

Edited part of SwipeableList.js (see comments):

function ListItem({ item, onRemove }) {
  const isRemoving = useSharedValue(false);
  const translateX = useSharedValue(0);
  // Declare stateful value
  const [isSwiping, setIsSwiping] = useState(false);

  const handler = useAnimatedGestureHandler({
    onStart: (evt, ctx) => {
      ctx.startX = translateX.value;
      // Modify stateful value from worklet
      runOnJS(setIsSwiping)(true);
    },

    onActive: (evt, ctx) => {
      const nextTranslate = evt.translationX + ctx.startX;
      translateX.value = Math.min(0, Math.max(nextTranslate, MAX_TRANSLATE));
    },

    onEnd: (evt) => {
      if (evt.velocityX < -20) {
        translateX.value = withSpring(
          MAX_TRANSLATE,
          springConfig(evt.velocityX)
        );
      } else {
        translateX.value = withSpring(0, springConfig(evt.velocityX));
      }
    },
  });

// ... Rest of ListItem implementation. Truncated for clarity
}

I can provide a project on Github if it helps.

Package versions

• React: 16.13.1
• React Native: 0.63.0
• React Native Reanimated: Master @ 29491efc1851df320c2fa1ba90b5c217bd7a22cc (Nov 30th)
• react-native-gesture-handler: 1.8.0
• NodeJS: 12.16.1

[github-actions[bot]]

Issue validator - update # 1

Hello!
Congratulations! Your issue passed the validator! Thank you!

[karol-bisztyga]

Hello 👋 I just want to note that useState doesn't seem to play a role here. It looks like there's a problem but for me, such change in code suffices:

export default function SwipableList() {
  function onRemove() {
    alert('Removed');
  }

  for (let i=0;i<1000;++i) {
    data.push({ id: `${i + 8}`, title: 'dummy item ' + i });
  }

  return (
    <View style={s.container}>
      <FlatList
        data={data}
        renderItem={({ item }) => <ListItem item={item} onRemove={onRemove} />}
        keyExtractor={(item) => item.id}
      />
    </View>
  );
}

Just creating a lot of dummy elements and then navigating back and forth does the job, the memory increases significantly until the app is reloaded.

[Szymon20000]

I think useState creates a problem here. Actually, capturing setState function does as it has ref to FiberNode and as a result to the PanGestureHandler what creates a reference loop.

[Szymon20000]

Here is a minimal code example I found:

import React, {useState} from 'react';
import { View, Text } from 'react-native';
import Animated, { useAnimatedGestureHandler, runOnJS } from 'react-native-reanimated';
import { PanGestureHandler } from 'react-native-gesture-handler';

export default function App() {
  const [state, setState] = useState(false);

  const gh = useAnimatedGestureHandler({
    onStart:() => {
      runOnJS(setState)(true);
    },
  })

  return (
    <View style={{flex:1, backgroundColor: 'yellow'}}>
      <PanGestureHandler onGestureEvent={gh}>
        <Animated.View>
          <Text>test</Text>
        </Animated.View>
      </PanGestureHandler>
    </View>
  );
}

[sparga]

@Szymon20000 I think you're right, useState is probably not the actual problem here. I managed to reproduce the leak by just declaring a function in the component and using it in the worklet.
Does it mean that all functions used in a worklet must be declared outside the component? Intuitively, I was expecting useAnimatedGestureHandler to behave like any other hook.

[karol-bisztyga]

@Szymon20000 I think you're right, useState is probably not the actual problem here. I managed to reproduce the leak by just declaring a function in the component and using it in the worklet.
Does it mean that all functions used in a worklet must be declared outside the component? Intuitively, I was expecting useAnimatedGestureHandler to behave like any other hook.

@sparga I'm confused. First I wrote that useState doesn't play a role here, by what I meant it doesn't cause the problem itself. Then @Szymon20000 wrote that indeed useState is the cause of the problem. Finally, you replied that Szymon's right - useState is not a problem.

Is it just me having a weird day or something doesn't make sense here?

[sylvain-abadie]

Chiming in for @sparga here since he's on another timezone. Calling setState from the worklet of the AnimatedGestureHandler triggers the leak. The minimal example @Szymon20000 wrote is correct.

I think you can disregard @sparga 's latest comment for now. He'll clarify what he meant later.

[karol-bisztyga]

Sure, no problem, I just didn't understand 😉

[sparga]

Sorry, I should have been clearer, forgot my previous message.

Yes, you're right @karol-bisztyga, useState doesn't play a direct role. I simply meant that using setState in the worklet is what causes the leak.

Here is another example similar to the first one (without any use of any state this time 😅):

function ListItem({ item, onRemove }) {
  const completeSwipe = () => {
    onRemove()
  }
// ... Rest of ListItem implementation. Truncated for clarity 
}

If at any point in a worklet declared in the ListItem function you call runOnJS(completeSwipe)();, it creates some kind of reference cycle and the PanGestureHandlers and their children leak.

---

I also tried your code and while memory usage is increasing it looks like a different issue because the PanGestureHandlers are correctly deallocated in that case.

Let me know if it's still unclear. I can provide a sample project if needed. Thank you!

[karol-bisztyga]

Today I discovered that it could just be a problem of the gesture handler itself. Such code leaks memory:

import React from 'react';
import { View, Text } from 'react-native';
import { PanGestureHandler } from 'react-native-gesture-handler';

export default function App() {

  const handler = (e) => {};

  return (
    <View>
      <PanGestureHandler activeOffsetX={[-10, 10]} onGestureEvent={handler}>
        <Text>AAA</Text>
      </PanGestureHandler>
    </View>
  );
}

It has nothing to do with reanimated.

We will let you know when we check the GH.

[karol-bisztyga]

Hey!
All the problems with memory leaks that we detected in reanimated 2 have been solved here

We also noticed that for some cases the leaks don't really happen - the dangling objects wait for the GC on the UI side which works separately from the point of view of that one on the JS side - the information about this is in the PR.

As for the GH: I didn't reproduce it now but when I do I'll create an issue in their repo.

Thanks for bringing this up 🙌

[sylvain-abadie]

Thanks @karol-bisztyga 🙌 ! We'll test this out ASAP!

[karol-bisztyga]

The release containing this change will come out soon, in the meantime, you can use the daily builds