Skip to content

Commit

Permalink
Synchronize access to ViewManagerRegistry
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javache authored and pull[bot] committed Mar 28, 2023
1 parent 4a2d125 commit 8341192
Showing 1 changed file with 15 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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;

Expand Down Expand Up @@ -51,7 +52,7 @@ public ViewManagerRegistry(Map<String, ViewManager> viewManagerMap) {
* 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;
Expand Down Expand Up @@ -84,7 +85,7 @@ public ViewManager get(String className) {
* 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;
Expand All @@ -97,12 +98,16 @@ ViewManager getViewManagerIfExists(String className) {

/** Send lifecycle signal to all ViewManagers that StopSurface has been called. */
public void onSurfaceStopped(final int surfaceId) {
List<ViewManager> viewManagers;
synchronized (this) {
viewManagers = new ArrayList<>(mViewManagers.values());
}
Runnable runnable =
new Runnable() {
@Override
public void run() {
for (Map.Entry<String, ViewManager> entry : mViewManagers.entrySet()) {
entry.getValue().onSurfaceStopped(surfaceId);
for (ViewManager viewManager : viewManagers) {
viewManager.onSurfaceStopped(surfaceId);
}
}
};
Expand All @@ -116,12 +121,16 @@ public void run() {
/** ComponentCallbacks2 method. */
@Override
public void onTrimMemory(int level) {
List<ViewManager> viewManagers;
synchronized (this) {
viewManagers = new ArrayList<>(mViewManagers.values());
}
Runnable runnable =
new Runnable() {
@Override
public void run() {
for (Map.Entry<String, ViewManager> entry : mViewManagers.entrySet()) {
entry.getValue().trimMemory();
for (ViewManager viewManager : viewManagers) {
viewManager.trimMemory();
}
}
};
Expand Down

0 comments on commit 8341192

Please sign in to comment.