Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SurfaceMountingManager leaking views from stopped surfaces #37964

Closed
wants to merge 1 commit into from
Closed

Fix SurfaceMountingManager leaking views from stopped surfaces #37964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,7 @@ public class ReactFeatureFlags {

/** Report mount operations from the host platform to notify mount hooks. */
public static boolean enableMountHooks = false;

/** Fixes a leak in SurfaceMountingManager.mTagSetForStoppedSurface */
public static boolean fixStoppedSurfaceTagSetLeak = true;
}
53 changes: 35 additions & 18 deletions ...ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import androidx.annotation.AnyThread;
import androidx.annotation.NonNull;
import androidx.annotation.UiThread;
import androidx.collection.SparseArrayCompat;
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.infer.annotation.ThreadConfined;
Expand Down Expand Up @@ -52,6 +53,7 @@
import com.facebook.react.views.view.ReactViewManagerWrapper;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Map;
import java.util.Queue;
import java.util.Set;
import java.util.Stack;
Expand Down Expand Up @@ -91,7 +93,8 @@ public class SurfaceMountingManager {
private RemoveDeleteTreeUIFrameCallback mRemoveDeleteTreeUIFrameCallback;

// This is null *until* StopSurface is called.
private Set<Integer> mTagSetForStoppedSurface;
private Set<Integer> mTagSetForStoppedSurfaceLegacy;
private SparseArrayCompat<Object> mTagSetForStoppedSurface;

private final int mSurfaceId;

Expand Down Expand Up @@ -166,7 +169,10 @@ public boolean getViewExists(int tag) {
// If Surface stopped, check if tag *was* associated with this Surface, even though it's been
// deleted. This helps distinguish between scenarios where an invalid tag is referenced, vs
// race conditions where an imperative method is called on a tag during/just after StopSurface.
if (mTagSetForStoppedSurface != null && mTagSetForStoppedSurface.contains(tag)) {
if (mTagSetForStoppedSurface != null && mTagSetForStoppedSurface.containsKey(tag)) {
return true;
}
if (mTagSetForStoppedSurfaceLegacy != null && mTagSetForStoppedSurfaceLegacy.contains(tag)) {
return true;
}
if (mTagToViewState == null) {
Expand Down Expand Up @@ -287,27 +293,38 @@ public void stopSurface() {
}

Runnable runnable =
new Runnable() {
@Override
public void run() {
// We must call `onDropViewInstance` on all remaining Views
() -> {
if (ReactFeatureFlags.fixStoppedSurfaceTagSetLeak) {
mTagSetForStoppedSurface = new SparseArrayCompat<>();
for (Map.Entry<Integer, ViewState> entry : mTagToViewState.entrySet()) {
// Using this as a placeholder value in the map. We're using SparseArrayCompat
// since it can efficiently represent the list of pending tags
mTagSetForStoppedSurface.put(entry.getKey(), this);

// We must call `onDropViewInstance` on all remaining Views
onViewStateDeleted(entry.getValue());
}
} else {
for (ViewState viewState : mTagToViewState.values()) {
// We must call `onDropViewInstance` on all remaining Views
onViewStateDeleted(viewState);
}
mTagSetForStoppedSurfaceLegacy = mTagToViewState.keySet();
}

// Evict all views from cache and memory
mTagSetForStoppedSurface = mTagToViewState.keySet();
mTagToViewState = null;
mJSResponderHandler = null;
mRootViewManager = null;
mMountItemExecutor = null;
mOnViewAttachItems.clear();

if (ReactFeatureFlags.enableViewRecycling) {
mViewManagerRegistry.onSurfaceStopped(mSurfaceId);
}
FLog.e(TAG, "Surface [" + mSurfaceId + "] was stopped on SurfaceMountingManager.");
// Evict all views from cache and memory
// TODO: clear instead of nulling out to simplify null-safety in this class
mTagToViewState = null;
mJSResponderHandler = null;
mRootViewManager = null;
mMountItemExecutor = null;
mThemedReactContext = null;
mOnViewAttachItems.clear();

if (ReactFeatureFlags.enableViewRecycling) {
mViewManagerRegistry.onSurfaceStopped(mSurfaceId);
}
FLog.e(TAG, "Surface [" + mSurfaceId + "] was stopped on SurfaceMountingManager.");
};

if (UiThreadUtil.isOnUiThread()) {
Expand Down