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
This commit is contained in:
Pieter De Baets
2024-10-31 12:33:13 -07:00
committed by Facebook GitHub Bot
parent e5dd7d68bf
commit 6ba7cb3102
@@ -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