Skip to content

Commit bc766ec

Browse files
yinfeifacebook-github-bot
authored andcommitted
fix memory leak in Android (#35889)
Summary: fix memory leak in Android The issues link: #35890 Explain Defect since 0.65, when call dispatchViewUpdates, we add judge mNumRootViews>0 <img width="531" alt="image" src="https://user-images.githubusercontent.com/20136688/213394240-4c284df4-27de-494e-8eed-8e5f30796cce.png"> When we call removeRootView, it will decrease the mNumRootViews before dispatchViewUpdates <img width="430" alt="image" src="https://user-images.githubusercontent.com/20136688/213394611-d47ab49f-fc7a-4dd1-9836-fe667d655660.png"> 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 <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> Pull Request resolved: #35889 Reviewed By: christophpurrer Differential Revision: D42636805 Pulled By: sshic fbshipit-source-id: 76845b5c1fbdeaf1ebe990356e82059dc1e5b46e
1 parent 9eaf6f5 commit bc766ec

File tree

3 files changed

+25
-9
lines changed

3 files changed

+25
-9
lines changed

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,15 @@ public synchronized void removeRootView(int rootViewTag) {
666666
mRootTags.delete(rootViewTag);
667667
}
668668

669+
/**
670+
* Return root view num
671+
*
672+
* @return The num of root view
673+
*/
674+
public synchronized int getRootViewNum() {
675+
return mRootTags.size();
676+
}
677+
669678
/**
670679
* Returns true on success, false on failure. If successful, after calling, output buffer will be
671680
* {x, y, width, height}.

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,15 @@ public void removeRootView(int rootViewTag) {
170170
mOperationsQueue.enqueueRemoveRootView(rootViewTag);
171171
}
172172

173+
/**
174+
* Return root view num
175+
*
176+
* @return The num of root view
177+
*/
178+
private int getRootViewNum() {
179+
return mOperationsQueue.getNativeViewHierarchyManager().getRootViewNum();
180+
}
181+
173182
/** Unregisters a root node with a given tag from the shadow node registry */
174183
public void removeRootShadowNode(int rootViewTag) {
175184
synchronized (uiImplementationThreadLock) {
@@ -599,6 +608,12 @@ public void measureLayoutRelativeToParent(
599608

600609
/** Invoked at the end of the transaction to commit any updates to the node hierarchy. */
601610
public void dispatchViewUpdates(int batchId) {
611+
if (getRootViewNum() <= 0) {
612+
// If there are no RootViews registered, there will be no View updates to dispatch.
613+
// This is a hack to prevent this from being called when Fabric is used everywhere.
614+
// This should no longer be necessary in Bridgeless Mode.
615+
return;
616+
}
602617
SystraceMessage.beginSection(
603618
Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIImplementation.dispatchViewUpdates")
604619
.arg("batchId", batchId)

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ public interface CustomEventNamesResolver {
113113
private volatile int mViewManagerConstantsCacheSize;
114114

115115
private int mBatchId = 0;
116-
private int mNumRootViews = 0;
117116

118117
public UIManagerModule(
119118
ReactApplicationContext reactContext,
@@ -403,7 +402,6 @@ public <T extends View> int addRootView(
403402
-1);
404403

405404
mUIImplementation.registerRootView(rootView, tag, themedRootContext);
406-
mNumRootViews++;
407405
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
408406
return tag;
409407
}
@@ -427,7 +425,6 @@ public void stopSurface(final int surfaceId) {
427425
@ReactMethod
428426
public void removeRootView(int rootViewTag) {
429427
mUIImplementation.removeRootView(rootViewTag);
430-
mNumRootViews--;
431428
}
432429

433430
public void updateNodeSize(int nodeViewTag, int newWidth, int newHeight) {
@@ -768,12 +765,7 @@ public void onBatchComplete() {
768765
listener.willDispatchViewUpdates(this);
769766
}
770767
try {
771-
// If there are no RootViews registered, there will be no View updates to dispatch.
772-
// This is a hack to prevent this from being called when Fabric is used everywhere.
773-
// This should no longer be necessary in Bridgeless Mode.
774-
if (mNumRootViews > 0) {
775-
mUIImplementation.dispatchViewUpdates(batchId);
776-
}
768+
mUIImplementation.dispatchViewUpdates(batchId);
777769
} finally {
778770
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
779771
}

0 commit comments

Comments
 (0)