Addressing leaked Views in Fabric MountingManager

Summary:
MountingManager keeps a map of tags to Views, and attempts to clean it up by (1) deleting tags when they're explicitly deleted, (2) recursively deleting all Views when the View hierarchy is torn down.

However, there appear to be.... substantial gaps here. In tests, when navigating between screen X -> screen Y -> back to X (triggering a StopSurface), each "StopSurface" resulted in 200-600 Views being leaked (!).

What is causing these leaks? Well, for one, the "dropView" mechanism isn't perfect, so it might be missed Views. Second, Views don't always guarantee that `reactTag == getId()`, so that could result in leaks. Third, View preallocation on Android complicates things: Views can be preallocated and then never even inserted into the View hierarchy, so DELETE mutations could never be issued. Fourth, StopSurface is also complicated on Android (largely because of View preallocatioAndroid (largely because of View preallocation).

So, I introduce a new mechanism: keep a list of all tags for a surface, and remove all tags for a surface when the surface is torn down. This should be fool-proof: it handles preallocation and normal creation; it can handle deletes; and we're guaranteed that tags cannot be added after a surface is stopped.

Is this overly complicating things? Well, hopefully we can simplify all of this in the longterm. But until we get rid of View Preallocation, it seems like we need this mechanism - and View Preallocation might be around for a while, or forever.

Other thoughts: it's possible that using other data-structures could be more efficient, but I'm guessing the perf implications here are marginal (compared to the insane amount of memory leaks we're fixing). It could also simplify things to have a SurfaceMountingManager interface that implies all actions happen on a specific surface, including teardown.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D25985409

fbshipit-source-id: f55b533770b1630c6c2a9b7a694d953aa3324428

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java

@@ -42,6 +42,8 @@ import com.facebook.react.uimanager.ViewGroupManager;
 import com.facebook.react.uimanager.ViewManager;
 import com.facebook.react.uimanager.ViewManagerRegistry;
 import com.facebook.yoga.YogaMeasureMode;
+import java.util.LinkedHashMap;
+import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -52,13 +54,15 @@ public class MountingManager {
   public static final String TAG = MountingManager.class.getSimpleName();
   private static final boolean SHOW_CHANGED_VIEW_HIERARCHIES = ReactBuildConfig.DEBUG && false;
 
-  @NonNull private final ConcurrentHashMap<Integer, ViewState> mTagToViewState;
+  @NonNull private final ConcurrentHashMap<Integer, ViewState> mTagToViewState; // any thread
+  @NonNull private final LinkedHashMap<Integer, TreeSet<Integer>> mRootTagToTags; // UI thread only
   @NonNull private final JSResponderHandler mJSResponderHandler = new JSResponderHandler();
   @NonNull private final ViewManagerRegistry mViewManagerRegistry;
   @NonNull private final RootViewManager mRootViewManager = new RootViewManager();
 
   public MountingManager(@NonNull ViewManagerRegistry viewManagerRegistry) {
     mTagToViewState = new ConcurrentHashMap<>();
+    mRootTagToTags = new LinkedHashMap<>();
     mViewManagerRegistry = viewManagerRegistry;
   }
 
@@ -120,6 +124,7 @@ public class MountingManager {
                       + "explicitly overwrite the id field to View.NO_ID before calling addRootView.");
             }
             rootView.setId(reactRootTag);
+            mRootTagToTags.put(reactRootTag, new TreeSet<Integer>());
           }
         });
   }
@@ -131,6 +136,17 @@ public class MountingManager {
     if (rootViewState != null && rootViewState.mView != null) {
       dropView(rootViewState.mView, true);
     }
+
+    // Iterate through all tags of reactRootTag, delete all retained Views associated with tags
+    // Tags that could be left-over, even after `dropView` is called: PreAllocated Views that were
+    // never properly deleted, for example. There might be other causes of leaks as well.
+    // This doesn't remove Views from the View Hierarchy, it just ensures that we don't leak
+    // memory by holding onto native Views.
+    TreeSet<Integer> tags = mRootTagToTags.get(reactRootTag);
+    mRootTagToTags.remove(reactRootTag);
+    for (int tag : tags) {
+      mTagToViewState.remove(tag);
+    }
   }
 
   /** Releases all references to given native View. */
@@ -512,6 +528,7 @@ public class MountingManager {
   public void createView(
       @NonNull ThemedReactContext themedReactContext,
       @NonNull String componentName,
+      int rootTag,
       int reactTag,
       @Nullable ReadableMap props,
       @Nullable StateWrapper stateWrapper,
@@ -542,6 +559,7 @@ public class MountingManager {
     viewState.mCurrentState = (stateWrapper != null ? stateWrapper.getState() : null);
 
     mTagToViewState.put(reactTag, viewState);
+    mRootTagToTags.get(rootTag).add(reactTag);
   }
 
   @UiThread
@@ -622,7 +640,7 @@ public class MountingManager {
   }
 
   @UiThread
-  public void deleteView(int reactTag) {
+  public void deleteView(int rootTag, int reactTag) {
     UiThreadUtil.assertOnUiThread();
     ViewState viewState = getNullableViewState(reactTag);
 
@@ -643,6 +661,7 @@ public class MountingManager {
     // Additionally, as documented in `dropView`, we cannot always trust a
     // view's children to be up-to-date.
     mTagToViewState.remove(reactTag);
+    mRootTagToTags.get(rootTag).remove(reactTag);
 
     // For non-root views we notify viewmanager with {@link ViewManager#onDropInstance}
     ViewManager viewManager = viewState.mViewManager;
@@ -675,6 +694,7 @@ public class MountingManager {
   public void preallocateView(
       @NonNull ThemedReactContext reactContext,
       String componentName,
+      int rootTag,
       int reactTag,
       @Nullable ReadableMap props,
       @Nullable StateWrapper stateWrapper,
@@ -685,7 +705,12 @@ public class MountingManager {
           "View for component " + componentName + " with tag " + reactTag + " already exists.");
     }
 
-    createView(reactContext, componentName, reactTag, props, stateWrapper, isLayoutable);
+    // Views can be preallocated before the surface is started
+    if (mRootTagToTags.get(rootTag) == null) {
+      mRootTagToTags.put(rootTag, new TreeSet<Integer>());
+    }
+
+    createView(reactContext, componentName, rootTag, reactTag, props, stateWrapper, isLayoutable);
   }
 
   @UiThread

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java

@@ -140,12 +140,13 @@ public class IntBufferBatchMountItem implements MountItem {
           mountingManager.createView(
               mContext,
               componentName,
+              mRootTag,
               mIntBuffer[i++],
               castToProps(mObjBuffer[j++]),
               castToState(mObjBuffer[j++]),
               mIntBuffer[i++] == 1);
         } else if (type == INSTRUCTION_DELETE) {
-          mountingManager.deleteView(mIntBuffer[i++]);
+          mountingManager.deleteView(mRootTag, mIntBuffer[i++]);
         } else if (type == INSTRUCTION_INSERT) {
           int tag = mIntBuffer[i++];
           int parentTag = mIntBuffer[i++];

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java

@@ -64,7 +64,7 @@ public class PreAllocateViewMountItem implements MountItem {
               + mRootTag);
     }
     mountingManager.preallocateView(
-        mContext, mComponent, mReactTag, mProps, mStateWrapper, mIsLayoutable);
+        mContext, mComponent, mRootTag, mReactTag, mProps, mStateWrapper, mIsLayoutable);
   }
 
   @Override