Memory leak in `useSharedValue`

[Lapz]

Description

The memory used by a useSharedValue is never released when the component is unmounted and is always held

Steps to reproduce

Create a flatlist component that renders some subcomponents, the subcomponents all useSharedValues. Swipe through the list and you'll see that the amount of memory keeps on increasing and it's never released. Running the hermes profiler and you'll see that the useSharedValue comes up a lot with the number of allocations. I've attached a repo below where you can take a look, this seems to be similar to #5614. I can add more information if needed

I've also included a video, if you look at the memory usage, you'll see it continuously increase and never go down

https://github.com/software-mansion/react-native-reanimated/assets/19998186/890b619f-4dd8-4b8f-9c3a-76c4e5119f3f

Snack or a link to a repository

https://github.com/Lapz/reanimated-shared-values-leak

Reanimated version

3.8.0

React Native version

0.73.5

Platforms

iOS

JavaScript runtime

Hermes

Workflow

Expo Go

Architecture

Paper (Old Architecture)

Build type

None

Device

iOS simulator

Device model

No response

Acknowledgements

Yes

[MehediH]

I am also experiencing the same, @piaskowyk would be great if you or anyone from the team could kindly investigate. Thank you so much! 🙇🏽

[efstathiosntonas]

On the latest nightly (01 April '24) running the example on iOS 17.2 using Xcode 15.2:

1. Run the official ReanimatedExample
2. Tap RuntimeTestsExample row
3. Tap Run tests
4. Let it run for a while or wait for it to finish.

On terminal:

leaks ReanimatedExample 

results in:

Analysis Tool Version:  iOS Simulator 17.2 (21C62)

Physical footprint:         430.7M
Physical footprint (peak):  461.8M
Idle exit:                  untracked
----

leaks Report Version: 4.0
Process 12646: 232495 nodes malloced for 357758 KB
Process 12646: 422 leaks for 9792 total leaked bytes.

    422 (9.56K) << TOTAL >>
      1 (1008 bytes) ROOT LEAK: 0x108973580 [1008]  length: 9  "REAModule"
      1 (1008 bytes) ROOT LEAK: 0x1089769b0 [1008]  length: 34  "reanimated::NativeReanimatedModule"
      1 (1008 bytes) ROOT LEAK: 0x12d49e610 [1008]  length: 9  "REAModule"
...
...
...

trying to trace the leak: leaks --traceTree=0x12d49e610 ReanimatedExample results to:

Tracing: 0x12d49e610 [1008]  length: 9  "REAModule"

Found 0 roots referencing: 0x12d49e610 [1008]  length: 9  "REAModule"

This is a reverse reference tree showing how the specified block is referenced.
The top node is the specified block, the next level down are blocks that reference that, and so on.
The number at the start of each line is the number of paths-to-roots going through that node.
This reverse reference tree can be imported into Instruments using Import Sample Data to allow data mining.

Not quite sure why it fails to trace it, seems it needs a .memgraph for this, running it with --trace though brings up the tree but I won't paste it here, it's huge.

By using Instruments Leaks will find 17 leaks after running the tests with much more info to trace down.

After tests are finished the memory is not released.

It seems that is leaking from here:

Click me for Instruments screenshots

 

SingleInstanceChecker<T>::SingleInstanceChecker() {
  int status = 0;
  std::string className =
      __cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status);

  // Only one instance should exist, but it is possible for two instances
  // to co-exist during a reload.
  assertWithMessage(
      instanceCount_ <= 1,
      "[Reanimated] More than one instance of " + className +
          " present. This may indicate a memory leak due to a retain cycle.");

  instanceCount_++;
}

[efstathiosntonas]

@tjzel @piaskowyk can you please investigate? Some memory leak issues have been risen lately :/

In my lists the memory spikes at 1.5GB on an iPhone 15 Pro Max after 150-200 items in FlashList, I don't want to imagine what happens with low end devices 😅

[efstathiosntonas]

my knowledge on C: 0

I asked ChatGPT about the SingleInstanceChecker.h under Common/cpp/Tools to suggest some improvements and it gave me back this:

Click me

```c #pragma once #ifndef NDEBUG #include #include #include #include #ifdef ANDROID #include #endif namespace reanimated { // This is a class that counts how many instances of a different class there // are. It is meant only to be used with classes that should only have one // instance. template class SingleInstanceChecker { public: SingleInstanceChecker(); ~SingleInstanceChecker(); private: void assertWithMessage(bool condition, std::string message) { if (!condition) { #ifdef ANDROID __android_log_print( ANDROID_LOG_WARN, "Reanimated", "%s", message.c_str()); #else std::cerr << "[Reanimated] " << message << std::endl; #endif #ifdef IS_REANIMATED_EXAMPLE_APP assert(false); #endif } } // A static field will exist separately for every class template. // This automatically initializes the inline variable. inline static std::atomic instanceCount_{0}; }; template SingleInstanceChecker::SingleInstanceChecker() { int status = 0; char* demangled_name = __cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status); std::string className = (status == 0 && demangled_name != nullptr) ? demangled_name : typeid(T).name(); assertWithMessage( instanceCount_.load() <= 1, "[Reanimated] More than one instance of " + className + " present. This may indicate a memory leak due to a retain cycle."); instanceCount_.fetch_add(1); free(demangled_name); } template SingleInstanceChecker::~SingleInstanceChecker() { instanceCount_.fetch_sub(1); } } // namespace reanimated #endif // NDEBUG ``` in patch format (v3.8.1): ```patch diff --git a/node_modules/react-native-reanimated/Common/cpp/Tools/SingleInstanceChecker.h b/node_modules/react-native-reanimated/Common/cpp/Tools/SingleInstanceChecker.h index 2a183e8..8fc21d6 100644 --- a/node_modules/react-native-reanimated/Common/cpp/Tools/SingleInstanceChecker.h +++ b/node_modules/react-native-reanimated/Common/cpp/Tools/SingleInstanceChecker.h @@ -41,29 +41,31 @@ class SingleInstanceChecker { } // A static field will exist separately for every class template. - // This has to be inline for automatic initialization. - inline static std::atomic instanceCount_; + // This automatically initializes the inline variable. + inline static std::atomic instanceCount_{0}; }; template SingleInstanceChecker::SingleInstanceChecker() { int status = 0; + char* demangled_name = __cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status); + std::string className = - __cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status); + (status == 0 && demangled_name != nullptr) ? demangled_name : typeid(T).name(); - // Only one instance should exist, but it is possible for two instances - // to co-exist during a reload. assertWithMessage( - instanceCount_ <= 1, + instanceCount_.load() <= 1, "[Reanimated] More than one instance of " + className + " present. This may indicate a memory leak due to a retain cycle."); - instanceCount_++; + instanceCount_.fetch_add(1); + + free(demangled_name); } template SingleInstanceChecker::~SingleInstanceChecker() { - instanceCount_--; + instanceCount_.fetch_sub(1); } } // namespace reanimated ```

The leaks are gone. I have no idea what I'm doing so I don't know if there are any side effects. Just thought it would be nice to share.

edit: the memory is still retained, at least it's not leaking.

[efstathiosntonas]

In a Flashlist with posts I use my lib to cache the images: https://github.com/georstat/react-native-image-cache

The lib transitions the thumbnail image with the actual image using timing so it's a heavy user of useAnimatedStyle, useSharedValue and withTiming when it comes to scrolling.

After 150-200 posts the RAM skyrockets to 1.5G as stated in a previous comment. The moment I switch the image component to expo-image without using any reanimated hook the RAM stays at 500MB.

[MehediH]

@efstathiosntonas You are correct. We were using react-native-fast-image and switching to Expo Image solved the memory leak issue. Now when components are unmounted, they actually release the memory.

Note that https://github.com/DylanVann/react-native-fast-image doesn't use reanimated, so I am not sure why using Expo Image fixes the issue for us.

[efstathiosntonas]

@MehediH this is a common issue with fast-image (that's the main reason I created the lib mentioned in the previous comment).

Problem is, the issue exists in the official ReanimatedExample too so it's unrelated to images component used, it just happens in my case were the cache image lib uses reanimated hooks heavily.

offtopic: expo-image defaults to disk caching, if you change the cachePolicy to memory you will see better results than fast-image though.

[devym-37]

Could this issue be related to the app freezing issue on Android with the useSharedValue issue ?

[PEZO19]

@efstathiosntonas Can you please update how your fix mentioned above is working? Have you used it since in a live project too? If so, how was the "integration" part of your fix?

[efstathiosntonas]

@PEZO19 hi, haven’t used it in production since I have no clue if what chatgpt proposed has any side effects in other parts of reanimated, I just posted that for testing purposes or to give some possible hints to SW devs.

[szydlovsky]

Hey everyone, I confirmed that we are aiming to take a look at this issue. However, it's not on the top of our priorities for now. It will probably come up in July. Stay tuned!

[avivash]

hi @szydlovsky 👋🏼 just wondering if your team has had a chance to look into this yet?

[szydlovsky]

Hey @avivash we are currently researching issues related to performance - but not this exact topic yet. If anything changes I will let you know! And, of course, I will always encourage trying to find the culprit yourself 😄