Skip to content

Commit

Permalink
fix memory leak in Android (facebook#35889)
Browse files Browse the repository at this point in the history
Summary:
fix memory leak in Android
The issues link: facebook#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: facebook#35889

Reviewed By: christophpurrer

Differential Revision: D42636805

Pulled By: sshic

fbshipit-source-id: 76845b5c1fbdeaf1ebe990356e82059dc1e5b46e
  • Loading branch information
yinfei authored and OlimpiaZurek committed May 22, 2023
1 parent 3f34fbe commit 981be4c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public interface CustomEventNamesResolver {
private volatile int mViewManagerConstantsCacheSize;

private int mBatchId = 0;
private int mNumRootViews = 0;

public UIManagerModule(
ReactApplicationContext reactContext,
Expand Down Expand Up @@ -403,7 +402,6 @@ public <T extends View> int addRootView(
-1);

mUIImplementation.registerRootView(rootView, tag, themedRootContext);
mNumRootViews++;
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
return tag;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 981be4c

Please sign in to comment.