From bc766ec7f8b18ddc0ff72a2fff5783eeeff24857 Mon Sep 17 00:00:00 2001 From: yinfei Date: Wed, 8 Feb 2023 13:35:48 -0800 Subject: [PATCH] fix memory leak in Android (#35889) Summary: fix memory leak in Android The issues link: https://github.com/facebook/react-native/issues/35890 Explain Defect since 0.65, when call dispatchViewUpdates, we add judge mNumRootViews>0 image When we call removeRootView, it will decrease the mNumRootViews before dispatchViewUpdates image So when we remove the last rootview, it will skip call dispatchViewUpdates and do not remove it, it will cause memory leak Explain Fix We should use the original root view num to judge, we can get it from NativeViewHierarchyManager ## Changelog [ANDROID] [FIXED] - fix memory leak in Android Pull Request resolved: https://github.com/facebook/react-native/pull/35889 Reviewed By: christophpurrer Differential Revision: D42636805 Pulled By: sshic fbshipit-source-id: 76845b5c1fbdeaf1ebe990356e82059dc1e5b46e --- .../uimanager/NativeViewHierarchyManager.java | 9 +++++++++ .../react/uimanager/UIImplementation.java | 15 +++++++++++++++ .../facebook/react/uimanager/UIManagerModule.java | 10 +--------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index d5fafeb70fc82c..2015936aab549d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -666,6 +666,15 @@ public synchronized void removeRootView(int rootViewTag) { mRootTags.delete(rootViewTag); } + /** + * Return root view num + * + * @return The num of root view + */ + public synchronized int getRootViewNum() { + return mRootTags.size(); + } + /** * Returns true on success, false on failure. If successful, after calling, output buffer will be * {x, y, width, height}. diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java index 55939a9679dde9..b6cca3dee09ab7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -170,6 +170,15 @@ public void removeRootView(int rootViewTag) { mOperationsQueue.enqueueRemoveRootView(rootViewTag); } + /** + * Return root view num + * + * @return The num of root view + */ + private int getRootViewNum() { + return mOperationsQueue.getNativeViewHierarchyManager().getRootViewNum(); + } + /** Unregisters a root node with a given tag from the shadow node registry */ public void removeRootShadowNode(int rootViewTag) { synchronized (uiImplementationThreadLock) { @@ -599,6 +608,12 @@ public void measureLayoutRelativeToParent( /** Invoked at the end of the transaction to commit any updates to the node hierarchy. */ public void dispatchViewUpdates(int batchId) { + if (getRootViewNum() <= 0) { + // If there are no RootViews registered, there will be no View updates to dispatch. + // This is a hack to prevent this from being called when Fabric is used everywhere. + // This should no longer be necessary in Bridgeless Mode. + return; + } SystraceMessage.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIImplementation.dispatchViewUpdates") .arg("batchId", batchId) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 752586990b6ddb..c4635ffcad73a0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -113,7 +113,6 @@ public interface CustomEventNamesResolver { private volatile int mViewManagerConstantsCacheSize; private int mBatchId = 0; - private int mNumRootViews = 0; public UIManagerModule( ReactApplicationContext reactContext, @@ -403,7 +402,6 @@ public int addRootView( -1); mUIImplementation.registerRootView(rootView, tag, themedRootContext); - mNumRootViews++; Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); return tag; } @@ -427,7 +425,6 @@ public void stopSurface(final int surfaceId) { @ReactMethod public void removeRootView(int rootViewTag) { mUIImplementation.removeRootView(rootViewTag); - mNumRootViews--; } public void updateNodeSize(int nodeViewTag, int newWidth, int newHeight) { @@ -768,12 +765,7 @@ public void onBatchComplete() { listener.willDispatchViewUpdates(this); } try { - // If there are no RootViews registered, there will be no View updates to dispatch. - // This is a hack to prevent this from being called when Fabric is used everywhere. - // This should no longer be necessary in Bridgeless Mode. - if (mNumRootViews > 0) { - mUIImplementation.dispatchViewUpdates(batchId); - } + mUIImplementation.dispatchViewUpdates(batchId); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); }