Pull the coordinator that reported a transaction, as iOS does, in the Android pull model

With enableMountingCoordinatorPullModelAndroid, a commit tells Java only its surface id, and the UI thread looked the surface's MountingCoordinator up in the binding's surface registry. Commits a surface makes while it starts or stops, when it is not in the registry, were not mounted, and the legacy stopSurface(int) deadlocked on the registry lock.

The commit now keeps the MountingCoordinator that reported it until the UI thread pulls, and pullAndExecuteTransaction pulls from it, as iOS pulls from the coordinator its main-queue block captures. If a surface is restarted with the same id while the previous surface's commit waits to be pulled, the latest coordinator wins and a warning is logged. Java does not mount such a restarted surface anyway (MountingManager.startSurface keeps the stopped one). The registry and the push model are unchanged.

Author: bartlomiejbloniarz

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp

@@ -125,29 +125,12 @@ void FabricUIManagerBinding::pullAndExecuteTransaction(SurfaceId surfaceId) {
 
   std::shared_ptr<const MountingCoordinator> mountingCoordinator;
   {
-    std::shared_lock lock(surfaceHandlerRegistryMutex_);
-    auto iterator = surfaceHandlerRegistry_.find(surfaceId);
-    if (iterator == surfaceHandlerRegistry_.end()) {
+    std::scoped_lock lock(pullableMountingCoordinatorsMutex_);
+    auto pullable = pullableMountingCoordinators_.extract(surfaceId);
+    if (pullable.empty()) {
       return;
     }
-    const auto* surfaceHandler = std::get_if<SurfaceHandler>(&iterator->second);
-    jni::local_ref<SurfaceHandlerBinding::jhybridobject> javaSurfaceHandler;
-    if (surfaceHandler == nullptr) {
-      javaSurfaceHandler =
-          std::get<jni::weak_ref<SurfaceHandlerBinding::jhybridobject>>(
-              iterator->second)
-              .lockLocal();
-      if (javaSurfaceHandler) {
-        surfaceHandler = &javaSurfaceHandler->cthis()->getSurfaceHandler();
-      }
-    }
-    if (surfaceHandler != nullptr) {
-      mountingCoordinator = surfaceHandler->getMountingCoordinator();
-    }
-  }
-
-  if (mountingCoordinator == nullptr) {
-    return;
+    mountingCoordinator = std::move(pullable.mapped());
   }
 
   auto mountingManager = getMountingManager("pullAndExecuteTransaction");
@@ -732,8 +715,19 @@ void FabricUIManagerBinding::schedulerShouldRenderTransactions(
     // commit thread). Just notify Java that a transaction is available; the UI
     // thread calls back into `pullAndExecuteTransaction` at its next mount
     // dispatch.
-    mountingManager->onTransactionAvailable(
-        mountingCoordinator->getSurfaceId());
+    auto surfaceId = mountingCoordinator->getSurfaceId();
+    {
+      std::scoped_lock lock(pullableMountingCoordinatorsMutex_);
+      auto& pullable = pullableMountingCoordinators_[surfaceId];
+      if (pullable != nullptr && pullable != mountingCoordinator) {
+        LOG(WARNING)
+            << "FabricUIManagerBinding::schedulerShouldRenderTransactions: "
+            << "surface " << surfaceId
+            << " was restarted while the previous surface's commit waits to be pulled";
+      }
+      pullable = mountingCoordinator;
+    }
+    mountingManager->onTransactionAvailable(surfaceId);
   } else if (ReactNativeFeatureFlags::
                  enableAccumulatedUpdatesInRawPropsAndroid()) {
     auto mountingTransaction = mountingCoordinator->pullTransaction(

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h

@@ -167,6 +167,12 @@ class FabricUIManagerBinding : public jni::HybridClass<FabricUIManagerBinding>,
   std::mutex pendingTransactionsMutex_;
   std::vector<MountingTransaction> pendingTransactions_;
 
+  // Pull model: keeps the coordinator that reported a surface's commit alive
+  // until the UI thread handles the pull, as iOS does by capturing it in the
+  // block it dispatches to the main queue. Java only gets the surface id.
+  std::mutex pullableMountingCoordinatorsMutex_;
+  std::unordered_map<SurfaceId, std::shared_ptr<const MountingCoordinator>> pullableMountingCoordinators_;
+
   float pointScaleFactor_ = 1;
 
   bool enableFabricLogs_{false};