From 17f4c45eed420f2a0f8fc17a4e145991dcc279ba Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 2 Oct 2024 05:10:49 -0700 Subject: [PATCH] Add missing lock in ReactInstanceManager#attachRootView (#46776) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46776 All other paths that touch `mAttachedReactRoots` uses this lock. We expect this to address a crash we're seeing in production where `startSurface` is invoked multiple times. Changelog: [Internal] Reviewed By: fabriziocucci Differential Revision: D63752083 fbshipit-source-id: f06d07aa70719945f6956fbf6e0cde3e9c8e8ed0 --- .../facebook/react/ReactInstanceManager.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 31cf7f73823..4730646870c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -942,22 +942,23 @@ public class ReactInstanceManager { @ThreadConfined(UI) public void attachRootView(ReactRoot reactRoot) { UiThreadUtil.assertOnUiThread(); + synchronized (mAttachedReactRoots) { + // Calling clearReactRoot is necessary to initialize the Id on reactRoot + // This is necessary independently if the RN Bridge has been initialized or not. + // Ideally reactRoot should be initialized with id == NO_ID + if (mAttachedReactRoots.add(reactRoot)) { + clearReactRoot(reactRoot); + } else { + FLog.e(ReactConstants.TAG, "ReactRoot was attached multiple times"); + } - // Calling clearReactRoot is necessary to initialize the Id on reactRoot - // This is necessary independently if the RN Bridge has been initialized or not. - // Ideally reactRoot should be initialized with id == NO_ID - if (mAttachedReactRoots.add(reactRoot)) { - clearReactRoot(reactRoot); - } else { - FLog.e(ReactConstants.TAG, "ReactRoot was attached multiple times"); - } - - // If react context is being created in the background, JS application will be started - // automatically when creation completes, as reactRoot is part of the attached - // reactRoot list. - ReactContext currentContext = getCurrentReactContext(); - if (mCreateReactContextThread == null && currentContext != null) { - attachRootViewToInstance(reactRoot); + // If react context is being created in the background, JS application will be started + // automatically when creation completes, as reactRoot is part of the attached + // reactRoot list. + ReactContext currentContext = getCurrentReactContext(); + if (mCreateReactContextThread == null && currentContext != null) { + attachRootViewToInstance(reactRoot); + } } }