Preventing/detecting memory leaks on Android

[lukmccall]

Introduction

In Expo, we recently discovered a couple of memory leaks. We identify two patterns that can lead to leaks affecting overall performance.

This problem may seem quite niche, as it initially appears to affect only a small group. However, it can significantly impact the DX and brownfield projects. In greenfield projects, this issue arises only when the OTA reloads the application.

It’s impossible to prevent users from writing code that leaks their own objects, which is fine. However, I think it would be beneficial to check if objects from React Native are not leaking. For instance, we discovered this problem in Expo Go because we observed multiple BridgeReactContext objects in the heap dump after reloading the app.

Details

Pattern 1 - Holding the jni::global_ref to the Java part of the HybridClass in the C++ code.

In fbjni, the Java part of the HybridClass is responsible for clearing the memory (when the GC removes it during de-initialization, it clears the C++ part). Generally speaking, this is safe and follows users’ expectations. However, when you need to call a Java method from C++ (for instance, when someone calls a JS method that needs to invoke Android-specific APIs), developers must create a global_ref to the Java object. This often results in a Java object that stores a C++ counterpart, which holds a global_ref to the (self) Java object. The JVM can handle circular references, but the jni::global_ref is treated as a static field; it is always accessible from the GC root and cannot be removed. Therefore, the user must manually remove one end of the references.

Let’s look at an example:

• In the react-native-reanimated, is a HybricClass called AndroidUIScheduler. The cpp code holds the reference to the Java part here. If you go through the file, you won’t see any place where this global reference is cleared. I don’t mind if the AndroidUIScheduler leaks; the library authors should be responsible for fixing and preventing it in the future. However, it also holds the reference to the ReactApplicationContext - https://github.com/software-mansion/react-native-reanimated/blob/8f30f1d807e6d0991e83fd7948c78a9621b08173/packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/AndroidUIScheduler.java#L16, reviling a bigger problem.

I don’t have a clear answer on how to fix this, but it would be helpful to have a mechanism for detecting if objects like the ReactApplicationContext are leaking.

Pattern 2 - not removing the listeners

It’s unclear to people if listeners should be removed or not. React Native has many different listeners, and many people are using them but forget to unsubscribe during de-initialization. Yes, in most cases, they will be cleared automatically. Although it isn’t hard to write a listener that captures much more than you think, causing weird leaks. I think it would be better to add some kind of warning to inform people if there is a dangling listener forcing developers to remove listeners in the clean-up phase

Discussion points

Because of C++, memory leaks are even more challenging to track and fix. We lack tools to detect bad code, and the connection between C++ and Java via JNI is often tricky to get right. We can't change how it works, but maybe we can structure the React Native code to mitigate some negative aspects, improving the overall experience.

• Should we add some mechanism to detect if objects from React Native are leaking?
  • if so, how should it work?
• Should we force people to unregister listeners?

[cortinico]

I don’t have a clear answer on how to fix this, but it would be helpful to have a mechanism for detecting if objects like the ReactApplicationContext are leaking.

@lukmccall Have you tried using LeakCanary https://square.github.io/leakcanary/ to see if we can automatically track those kind of issues?

[lukmccall]

We used it at the Expo for a while, but due to react-native-screens, it produces many false positives. However, I didn't use it to detect that particular issue.

[javache]

For pattern 1, we solved a number of issues like this in React Native earlier this year, eg facebook/react-native#43613 facebook/react-native#43611 facebook/react-native#43614 - sometimes the fix is just to remove the global_ref, but weak_ref can be a solution here too.