[repro] Pull model: the stop transaction is not pulled, and the legacy stop deadlocks

With enableMountingCoordinatorPullModelAndroid, FabricUIManagerBinding::pullAndExecuteTransaction finds the surface's MountingCoordinator in the binding's surface registry. A surface is not in the registry while it stops:

- The transaction that stops a bridgeless surface is never pulled. native_stopSurfaceWithSurfaceHandler_pullModel_pullsDeletes records no Deletes.
- The legacy stopSurface(int) holds the registry lock across SurfaceHandler::stop(), and on the UI thread the stop commit pulls inline and blocks on the same lock. native_stopSurface_pullModel_pullsDeletes hangs.

native_stopSurfaceWithSurfaceHandler_pushModel_pullsDeletes is a control: push pulls the stop transaction in the stop commit.

The tests drive a real FabricUIManagerBinding through FabricMountingManagerTestHelper, which runs no JavaScript and commits views itself.

Author: bartlomiejbloniarz

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

@@ -7,14 +7,54 @@
 
 #include "FabricMountingManagerTestHelper.h"
 
+#include <react/fabric/FabricUIManagerBinding.h>
 #include <react/renderer/animationbackend/AnimatedProps.h>
+#include <react/renderer/componentregistry/ComponentDescriptorProviderRegistry.h>
+#include <react/renderer/components/root/RootShadowNode.h>
 #include <react/renderer/components/view/ViewComponentDescriptor.h>
 #include <react/renderer/components/view/ViewProps.h>
 #include <react/renderer/core/ComponentDescriptor.h>
+#include <react/renderer/mounting/MountingOverrideDelegate.h>
 #include <react/renderer/mounting/ShadowView.h>
+#include <react/renderer/scheduler/Scheduler.h>
 
 namespace facebook::react {
 
+class FabricMountingManagerTestHelper::DeleteRecorder
+    : public MountingOverrideDelegate {
+ public:
+  bool shouldOverridePullTransaction() const override {
+    return true;
+  }
+
+  std::optional<MountingTransaction> pullTransaction(
+      SurfaceId surfaceId,
+      MountingTransaction::Number number,
+      const TransactionTelemetry& telemetry,
+      ShadowViewMutationList mutations) const override {
+    for (const auto& mutation : mutations) {
+      if (mutation.type == ShadowViewMutation::Delete) {
+        deletedTags.push_back(mutation.oldChildShadowView.tag);
+      }
+    }
+    return MountingTransaction{
+        surfaceId, number, std::move(mutations), telemetry};
+  }
+
+  mutable std::vector<Tag> deletedTags;
+};
+
+namespace {
+
+void runNoJavaScript(std::function<void(jsi::Runtime& runtime)>&& /*unused*/) {}
+
+std::vector<Tag> toTags(jni::alias_ref<jni::JArrayInt> tags) {
+  auto region = tags->getRegion(0, static_cast<jsize>(tags->size()));
+  return {region.get(), region.get() + tags->size()};
+}
+
+} // namespace
+
 FabricMountingManagerTestHelper::FabricMountingManagerTestHelper(
     jni::alias_ref<JFabricUIManager::javaobject> jFabricUIManager)
     : javaUIManager_(jni::make_global(jFabricUIManager)) {
@@ -81,6 +121,85 @@ void FabricMountingManagerTestHelper::synchronouslyUpdateAnimatedProps(
   mountingManager_->synchronouslyUpdateAnimatedProps(updates);
 }
 
+jni::local_ref<JRuntimeExecutor::jhybridobject>
+FabricMountingManagerTestHelper::createRuntimeExecutor() {
+  return JRuntimeExecutor::newObjectCxxArgs(runNoJavaScript);
+}
+
+jni::local_ref<JRuntimeScheduler::jhybridobject>
+FabricMountingManagerTestHelper::createRuntimeScheduler() {
+  // Work scheduled here never runs and stays queued. A queued task holds the
+  // UIManager, which holds this scheduler, so both outlive the test.
+  runtimeScheduler_ = std::make_shared<RuntimeScheduler>(runNoJavaScript);
+  return JRuntimeScheduler::newObjectCxxArgs(runtimeScheduler_);
+}
+
+void FabricMountingManagerTestHelper::registerViewComponent(
+    ComponentFactory* componentFactory) {
+  componentFactory->buildRegistryFunction =
+      [](const EventDispatcher::Weak& eventDispatcher,
+         const std::shared_ptr<const ContextContainer>& contextContainer) {
+        auto providerRegistry =
+            std::make_shared<ComponentDescriptorProviderRegistry>();
+        providerRegistry->add(
+            concreteComponentDescriptorProvider<ViewComponentDescriptor>());
+        return providerRegistry->createComponentDescriptorRegistry(
+            {.eventDispatcher = eventDispatcher,
+             .contextContainer = contextContainer,
+             .flavor = nullptr});
+      };
+}
+
+void FabricMountingManagerTestHelper::commitViews(
+    jint surfaceId,
+    jni::alias_ref<jni::JArrayInt> tags) {
+  auto children =
+      std::make_shared<std::vector<std::shared_ptr<const ShadowNode>>>();
+  for (auto tag : toTags(tags)) {
+    auto props = std::make_shared<ViewProps>();
+    props->collapsable = false;
+    children->push_back(viewComponentDescriptor_->createShadowNode(
+        {.props = props},
+        viewComponentDescriptor_->createFamily(
+            {.tag = tag, .surfaceId = surfaceId, .instanceHandle = nullptr})));
+  }
+  javaUIManager_->getBinding()
+      ->getScheduler()
+      ->getUIManager()
+      ->getShadowTreeRegistry()
+      .visit(surfaceId, [&](const ShadowTree& shadowTree) {
+        shadowTree.commit(
+            [&](const RootShadowNode& oldRootShadowNode) {
+              return std::make_shared<RootShadowNode>(
+                  oldRootShadowNode,
+                  ShadowNodeFragment{
+                      .props = ShadowNodeFragment::propsPlaceholder(),
+                      .children = children});
+            },
+            {});
+      });
+}
+
+void FabricMountingManagerTestHelper::recordPulledDeletes(jint surfaceId) {
+  deleteRecorder_ = std::make_shared<DeleteRecorder>();
+  javaUIManager_->getBinding()
+      ->getScheduler()
+      ->getUIManager()
+      ->getShadowTreeRegistry()
+      .visit(surfaceId, [&](const ShadowTree& shadowTree) {
+        shadowTree.getMountingCoordinator()->setMountingOverrideDelegate(
+            deleteRecorder_);
+      });
+}
+
+jni::local_ref<jni::JArrayInt>
+FabricMountingManagerTestHelper::pulledDeleteTags() {
+  const auto& tags = deleteRecorder_->deletedTags;
+  auto array = jni::JArrayInt::newArray(tags.size());
+  array->setRegion(0, static_cast<jsize>(tags.size()), tags.data());
+  return array;
+}
+
 void FabricMountingManagerTestHelper::registerNatives() {
   registerHybrid({
       makeNativeMethod(
@@ -99,6 +218,23 @@ void FabricMountingManagerTestHelper::registerNatives() {
       makeNativeMethod(
           "synchronouslyUpdateAnimatedProps",
           FabricMountingManagerTestHelper::synchronouslyUpdateAnimatedProps),
+      makeNativeMethod(
+          "createRuntimeExecutor",
+          FabricMountingManagerTestHelper::createRuntimeExecutor),
+      makeNativeMethod(
+          "createRuntimeScheduler",
+          FabricMountingManagerTestHelper::createRuntimeScheduler),
+      makeNativeMethod(
+          "registerViewComponent",
+          FabricMountingManagerTestHelper::registerViewComponent),
+      makeNativeMethod(
+          "commitViews", FabricMountingManagerTestHelper::commitViews),
+      makeNativeMethod(
+          "recordPulledDeletes",
+          FabricMountingManagerTestHelper::recordPulledDeletes),
+      makeNativeMethod(
+          "pulledDeleteTags",
+          FabricMountingManagerTestHelper::pulledDeleteTags),
   });
 }
 

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

@@ -8,9 +8,12 @@
 #pragma once
 
 #include <fbjni/fbjni.h>
+#include <react/jni/JRuntimeExecutor.h>
+#include <react/jni/JRuntimeScheduler.h>
 #include <react/jni/NativeMap.h>
 #include <react/renderer/components/view/ViewComponentDescriptor.h>
 
+#include <react/fabric/ComponentFactory.h>
 #include <react/fabric/FabricMountingManager.h>
 
 namespace facebook::react {
@@ -31,6 +34,17 @@ class FabricMountingManagerTestHelper : public jni::HybridClass<FabricMountingMa
       jni::alias_ref<jni::JArrayInt> tags,
       jni::alias_ref<jni::JArrayClass<NativeMap::javaobject>> props);
 
+  // A FabricUIManagerBinding registered on the helper's FabricUIManager with
+  // these never runs JavaScript: its surfaces mount only what the helper
+  // commits.
+  jni::local_ref<JRuntimeExecutor::jhybridobject> createRuntimeExecutor();
+  jni::local_ref<JRuntimeScheduler::jhybridobject> createRuntimeScheduler();
+  void registerViewComponent(ComponentFactory *componentFactory);
+
+  void commitViews(jint surfaceId, jni::alias_ref<jni::JArrayInt> tags);
+  void recordPulledDeletes(jint surfaceId);
+  jni::local_ref<jni::JArrayInt> pulledDeleteTags();
+
   static void registerNatives();
 
  private:
@@ -45,6 +59,9 @@ class FabricMountingManagerTestHelper : public jni::HybridClass<FabricMountingMa
   jni::global_ref<JFabricUIManager::javaobject> javaUIManager_;
   std::shared_ptr<FabricMountingManager> mountingManager_;
   std::unique_ptr<ViewComponentDescriptor> viewComponentDescriptor_;
+  std::shared_ptr<RuntimeScheduler> runtimeScheduler_;
+  class DeleteRecorder;
+  std::shared_ptr<DeleteRecorder> deleteRecorder_;
 };
 
 } // namespace facebook::react