Memory Leak in Dialog Management: ReactRootView not removed from View Tree when Dialog is not showing

[fannnnzhang]

Description

There is a memory leak in the dialog management code of React Native's new architecture. When the hide() method is called and the dialog is not currently showing, the ReactRootView is not removed from the view tree. This results in the ReactRootView holding a reference to the Activity, causing a memory leak.

Steps to reproduce

1. Create a dialog with a ReactRootView.
2. Call hide() on the dialog when it is not showing.
3. Observe that the ReactRootView is not removed from the view tree.

React Native Version

0.74.2

Affected Platforms

Runtime - Android

Areas

TurboModule - The New Native Module System

Output of npx react-native info

info Fetching system and libraries information...
System:
  OS: macOS 14.3
  CPU: (8) arm64 Apple M1
  Memory: 45.19 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 21.7.3
    path: /opt/homebrew/bin/node
  Yarn:
    version: 1.22.22
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.5.0
    path: /opt/homebrew/bin/npm
  Watchman:
    version: 2024.05.06.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.11.3
    path: /usr/local/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK:
    API Levels:
      - "25"
      - "26"
      - "27"
      - "28"
      - "29"
      - "30"
      - "31"
      - "32"
      - "33"
      - "33"
      - "34"
      - "34"
    Build Tools:
      - 26.0.2
      - 27.0.3
      - 28.0.2
      - 28.0.3
      - 29.0.2
      - 30.0.2
      - 30.0.3
      - 33.0.1
      - 34.0.0
    System Images:
      - android-34 | Google APIs ARM 64 v8a
    Android NDK: Not Found
IDEs:
  Android Studio: 2024.1 AI-241.15989.150.2411.11948838
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.1
    path: /usr/bin/javac
  Ruby:
    version: 3.3.2
    path: /opt/homebrew/opt/ruby/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native: Not Found
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: Not found
  newArchEnabled: Not found
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

(node:66274) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

Stacktrace or Logs

                                                                                                    │ GC Root: Global variable in native code
                                                                                                    │
                                                                                                    ├─ com.facebook.react.devsupport.LogBoxModule instance
                                                                                                    │    Leaking: UNKNOWN
                                                                                                    │    Retaining 40 B in 2 objects
                                                                                                    │    mReactApplicationContext instance of com.facebook.react.runtime.BridgelessReactContext, wrapping com.kk.
                                                                                                    │    MainApplication
                                                                                                    │    ↓ LogBoxModule.mSurfaceDelegate
                                                                                                    │                   ~~~~~~~~~~~~~~~~
                                                                                                    ├─ com.facebook.react.devsupport.LogBoxDialogSurfaceDelegate instance
                                                                                                    │    Leaking: UNKNOWN
                                                                                                    │    Retaining 20 B in 1 objects
                                                                                                    │    ↓ LogBoxDialogSurfaceDelegate.mReactRootView
                                                                                                    │                                  ~~~~~~~~~~~~~~
                                                                                                    ├─ com.facebook.react.runtime.ReactSurfaceView instance
                                                                                                    │    Leaking: YES (View.mContext references a destroyed activity)
                                                                                                    │    Retaining 1.2 kB in 18 objects
                                                                                                    │    View not part of a window view hierarchy
                                                                                                    │    View.mAttachInfo is null (view detached)
                                                                                                    │    View.mID = R.id.null
                                                                                                    │    View.mWindowAttachCount = 0
                                                                                                    │    mContext instance of com.kk.pages.RNActivity with mDestroyed = true
                                                                                                    │    ↓ View.mContext
                                                                                                    ╰→ com.kk.pages.RNActivity instance
                                                                                                    ​     Leaking: YES (ObjectWatcher was watching this because com.kk.pages.RNActivity received Activity#onDestroy()
                                                                                                    ​     callback and Activity#mDestroyed is true)
                                                                                                    ​     Retaining 78.1 kB in 857 objects
                                                                                                    ​     key = 45d74ebf-9e97-4545-90dd-1d24d3d975bc
                                                                                                    ​     watchDurationMillis = 25443
                                                                                                    ​     retainedDurationMillis = 20441
                                                                                                    ​     mApplication instance of com.kk.MainApplication
                                                                                                    ​     mBase instance of androidx.appcompat.view.ContextThemeWrapper

Reproducer

https://github.com/react-native-community/reproducer-react-native/tree/main

Screenshots and Videos

No response

[github-actions]

⚠️	Missing Reproducible Example
ℹ️	We could not detect a reproducible example in your issue report. Please provide either: If your bug is UI related: a Snack If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

[cortinico]

There is a memory leak in the dialog management code of React Native's new architecture. When the hide() method is called and the dialog is not currently showing, the ReactRootView is not removed from the view tree. This results in the ReactRootView holding a reference to the Activity, causing a memory leak.

Can you provide a valid repro an a screenshot from the memory profiler of Android Studio?

[fannnnzhang]

First, in my issue, I provided the memory leak path captured by LeakCanary. When looking at the code, this issue becomes very clear: Regardless of the state of the Dialog, mReactRootView will always be indirectly created and held by LogModule, and when ReactRootView is created, it holds a reference to the Activity. Therefore, the hide method of LogBoxDialogSurfaceDelegate must ensure that it is removed from the Activity. However, the current implementation of the hide method does not do this. When the dialog is not created or shown, it directly returns, causing the view to not release the Activity reference, which leads to a memory leak.

To reproduce this issue is very simple. You just need to start by creating a Native Activity in the React Native Android template project, then navigate to a ReactActivity, and finally return to the main Activity from the ReactActivity. This will reliably reproduce the memory leak. Additionally, this reproduction path may also uncover other memory leaks under the new architecture. I suggest conducting similar tests as soon as possible and fixing the related issues.
My local fix code like this:

  @Override
  public void hide() {
    if (isShowing()) {
      mDialog.dismiss();
    }

    if (mReactRootView != null && mReactRootView.getParent() != null) {
      ((ViewGroup) mReactRootView.getParent()).removeView(mReactRootView);
    }
    mDialog = null;
  }