From 6ba7cb310273a0ecce6a168bf3aae45fb85bf955 Mon Sep 17 00:00:00 2001
From: Pieter De Baets <pieterdb@meta.com>
Date: Thu, 31 Oct 2024 12:33:13 -0700
Subject: [PATCH] Fix potential race in FabricMountingManager (#47337)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47337

We call `onSurfaceStart` after calling `surfaceHandler.start()`. In theory it's possible for the React surface to start generating commits before we've informed the mounting manager of the surface having started, which could cause us to not accurately track views allocated for that surface so far.

Changelog: [Android][Fixed] Addressed race condition in surface start.

Reviewed By: rshest

Differential Revision: D65269674

fbshipit-source-id: 2927220bc17e61125dabcdd75f5325a0ca77e60b
---
 .../react/fabric/FabricUIManagerBinding.cpp   | 41 ++++++++-----------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp
index d5284596846..27976b921ba 100644
--- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp
+++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp
@@ -157,6 +157,11 @@ void FabricUIManagerBinding::startSurfaceWithSurfaceHandler(
   }
   scheduler->registerSurface(surfaceHandler);
 
+  auto mountingManager = getMountingManager("startSurfaceWithSurfaceHandler");
+  if (mountingManager != nullptr) {
+    mountingManager->onSurfaceStart(surfaceId);
+  }
+
   surfaceHandler.start();
 
   if (ReactNativeFeatureFlags::enableLayoutAnimationsOnAndroid()) {
@@ -167,14 +172,8 @@ void FabricUIManagerBinding::startSurfaceWithSurfaceHandler(
   {
     std::unique_lock lock(surfaceHandlerRegistryMutex_);
     surfaceHandlerRegistry_.emplace(
-        surfaceHandler.getSurfaceId(), jni::make_weak(surfaceHandlerBinding));
+        surfaceId, jni::make_weak(surfaceHandlerBinding));
   }
-
-  auto mountingManager = getMountingManager("startSurfaceWithSurfaceHandler");
-  if (!mountingManager) {
-    return;
-  }
-  mountingManager->onSurfaceStart(surfaceHandler.getSurfaceId());
 }
 
 // Used by non-bridgeless+Fabric
@@ -206,6 +205,11 @@ void FabricUIManagerBinding::startSurface(
 
   scheduler->registerSurface(surfaceHandler);
 
+  auto mountingManager = getMountingManager("startSurface");
+  if (mountingManager != nullptr) {
+    mountingManager->onSurfaceStart(surfaceId);
+  }
+
   surfaceHandler.start();
 
   if (ReactNativeFeatureFlags::enableLayoutAnimationsOnAndroid()) {
@@ -214,17 +218,9 @@ void FabricUIManagerBinding::startSurface(
   }
 
   {
-    SystraceSection s2("FabricUIManagerBinding::startSurface::surfaceId::lock");
     std::unique_lock lock(surfaceHandlerRegistryMutex_);
-    SystraceSection s3("FabricUIManagerBinding::startSurface::surfaceId");
     surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
   }
-
-  auto mountingManager = getMountingManager("startSurface");
-  if (!mountingManager) {
-    return;
-  }
-  mountingManager->onSurfaceStart(surfaceId);
 }
 
 // Used by non-bridgeless+Fabric
@@ -278,6 +274,11 @@ void FabricUIManagerBinding::startSurfaceWithConstraints(
 
   scheduler->registerSurface(surfaceHandler);
 
+  auto mountingManager = getMountingManager("startSurfaceWithConstraints");
+  if (mountingManager != nullptr) {
+    mountingManager->onSurfaceStart(surfaceId);
+  }
+
   surfaceHandler.start();
 
   if (ReactNativeFeatureFlags::enableLayoutAnimationsOnAndroid()) {
@@ -286,19 +287,9 @@ void FabricUIManagerBinding::startSurfaceWithConstraints(
   }
 
   {
-    SystraceSection s2(
-        "FabricUIManagerBinding::startSurfaceWithConstraints::surfaceId::lock");
     std::unique_lock lock(surfaceHandlerRegistryMutex_);
-    SystraceSection s3(
-        "FabricUIManagerBinding::startSurfaceWithConstraints::surfaceId");
     surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
   }
-
-  auto mountingManager = getMountingManager("startSurfaceWithConstraints");
-  if (!mountingManager) {
-    return;
-  }
-  mountingManager->onSurfaceStart(surfaceId);
 }
 
 // Used by non-bridgeless+Fabric