From 04496306123c46730b46accb2cd6239531e51fef Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Wed, 9 Oct 2024 05:25:10 -0700 Subject: [PATCH] Resolve Paper leak on Android (#46896) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46896 On Android Paper UIManager, when calling `ReactRootView.unmountReactApplication`, the ReactRootView tag is unset [here](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java#L926), before the round trip unmount operation to JS makes it's way back to a `dropView` call on `NativeViewHierarchyManager`. In practice, this means that legacy architecture apps that unmount surfaces via `ReactRootView.unmountReactApplication` leak references to Views, and the "finalization" step (`onDropViewInstance`) is not universally called. This is an attempt to fix the issue by skipping the `clearReactRoot` step on Paper, instead waiting for the round trip `UIManager.removeRootView` call. ## Changelog [Android][Fixed] Fix issue where `onDropViewInstance` cleanup was not being handled after `ReactRootView.unmountReactApplication` Reviewed By: javache Differential Revision: D64054042 fbshipit-source-id: b8b8c237796674ca23a332e57a1bf2e07ab5af13 --- .../main/java/com/facebook/react/ReactInstanceManager.java | 4 ++-- .../facebook/react/uimanager/NativeViewHierarchyManager.java | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 939f12c91ef..bc190d6f6e6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -1391,14 +1391,14 @@ public class ReactInstanceManager { new RuntimeException( "detachRootViewFromInstance called with ReactRootView with invalid id")); } + + clearReactRoot(reactRoot); } else { reactContext .getCatalystInstance() .getJSModule(AppRegistry.class) .unmountApplicationComponentAtRootTag(reactRoot.getRootViewTag()); } - - clearReactRoot(reactRoot); } @ThreadConfined(UI) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index 34ccb6f05bd..b5b1151317e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -674,6 +674,9 @@ public class NativeViewHierarchyManager { View rootView = mTagsToViews.get(rootViewTag); dropView(rootView); mRootTags.delete(rootViewTag); + if (rootView != null) { + rootView.setId(View.NO_ID); + } } /**