From a1f6b4d0ef6d85aa111df2995b093f66fb2df548 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 1 Mar 2023 04:37:05 -0800 Subject: [PATCH] Synchronize access to ViewManagerRegistry Summary: Found that we may do multiple allocations of the same ViewManager instance since ViewManagers are both accessed from the UI thread (mounting views) and from the Fabric background thread (for measuring Text), which could lead to multiple instances of the same ViewManager to be created. As far as I can tell, this issue was harmless since our ViewManager constructors don't have side-effects, but not ideal. Changelog: [Internall] Reviewed By: rshest Differential Revision: D43661306 fbshipit-source-id: 37ef82d41d43c334fdc6cfbeffb225bba87c668e --- .../react/uimanager/ViewManagerRegistry.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java index ec107e83b66..781f4f269d9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java @@ -12,6 +12,7 @@ import android.content.res.Configuration; import androidx.annotation.Nullable; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.MapBuilder; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -51,7 +52,7 @@ public final class ViewManagerRegistry implements ComponentCallbacks2 { * view manager registered for the className received as a parameter. * @return the {@link ViewManager} registered to the className received as a parameter */ - public ViewManager get(String className) { + public synchronized ViewManager get(String className) { ViewManager viewManager = mViewManagers.get(className); if (viewManager != null) { return viewManager; @@ -84,7 +85,7 @@ public final class ViewManagerRegistry implements ComponentCallbacks2 { * there is no ViewManager associated to the className received as a parameter. */ @Nullable - ViewManager getViewManagerIfExists(String className) { + /* package */ synchronized ViewManager getViewManagerIfExists(String className) { ViewManager viewManager = mViewManagers.get(className); if (viewManager != null) { return viewManager; @@ -97,12 +98,16 @@ public final class ViewManagerRegistry implements ComponentCallbacks2 { /** Send lifecycle signal to all ViewManagers that StopSurface has been called. */ public void onSurfaceStopped(final int surfaceId) { + List viewManagers; + synchronized (this) { + viewManagers = new ArrayList<>(mViewManagers.values()); + } Runnable runnable = new Runnable() { @Override public void run() { - for (Map.Entry entry : mViewManagers.entrySet()) { - entry.getValue().onSurfaceStopped(surfaceId); + for (ViewManager viewManager : viewManagers) { + viewManager.onSurfaceStopped(surfaceId); } } }; @@ -116,12 +121,16 @@ public final class ViewManagerRegistry implements ComponentCallbacks2 { /** ComponentCallbacks2 method. */ @Override public void onTrimMemory(int level) { + List viewManagers; + synchronized (this) { + viewManagers = new ArrayList<>(mViewManagers.values()); + } Runnable runnable = new Runnable() { @Override public void run() { - for (Map.Entry entry : mViewManagers.entrySet()) { - entry.getValue().trimMemory(); + for (ViewManager viewManager : viewManagers) { + viewManager.trimMemory(); } } };