From 629159a1e364ce5baaedbfab9758cc893979cf7c Mon Sep 17 00:00:00 2001
From: Alex Hunt <huntie@meta.com>
Date: Thu, 4 Apr 2024 07:17:56 -0700
Subject: [PATCH] Improve comment describing inspectorExecutor lifetime
 (#43856)

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

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D55744630

fbshipit-source-id: 4dc930342948253d4cc3b2d04fc4c474e53fc14d
---
 .../ReactCommon/cxxreact/Instance.cpp           | 17 +++++++++++------
 .../ReactCommon/cxxreact/Instance.h             |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.cpp b/packages/react-native/ReactCommon/cxxreact/Instance.cpp
index 06de3adb8e6..1cfcc06a56b 100644
--- a/packages/react-native/ReactCommon/cxxreact/Instance.cpp
+++ b/packages/react-native/ReactCommon/cxxreact/Instance.cpp
@@ -71,9 +71,14 @@ void Instance::initializeBridge(
       std::condition_variable inspectorInitializedCv;
       bool inspectorInitialized = false;
 
-      // Schedule work on the inspector thread. inspectorExecutor is guaranteed
-      // to execute this callback, so we can safely run this logic
-      // synchronously and reference parentInspectorTarget_.
+      // Schedule work on the inspector thread. NOTE: We expect this callback
+      // to always execute, given the invariant that `initializeBridge` (this
+      // method) completes before `unregisterFromInspector` is called.
+      // - On iOS, instance creation and invalidation both run on the main
+      //   queue (`RCTCxxBridge::start,invalidate` use `RCTAssertMainQueue`).
+      // - On Android, `ReactContext` must be initialized with a constructed
+      //   `CatalystInstance` (in which `Instance::initializeBridge` has
+      //   completed) before `destroy` can be called.
       inspectorExecutor([this,
                          &inspectorInitialized,
                          &inspectorInitializedMutex,
@@ -81,10 +86,10 @@ void Instance::initializeBridge(
                             jsinspector_modern::HostTarget& hostTarget) {
         // NOTE: By passing *this, we strongly assume the Instance will still
         // be alive by the time this executes.
-        // - On iOS, instance creation is done syncrhonously
-        //   (RCTCxxBridge#_initializeBridgeLocked).
+        // - On iOS, instance creation is done synchronously
+        //   (`RCTCxxBridge::_initializeBridgeLocked`).
         // - On Android, we explicitly wait for instance creation before
-        //   destruction (ReactInstanceManager#mReactContextLock).
+        //   destruction (`ReactInstanceManager::mReactContextLock`).
         inspectorTarget_ = &hostTarget.registerInstance(*this);
         RuntimeExecutor runtimeExecutorIfJsi = getRuntimeExecutor();
         runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.h b/packages/react-native/ReactCommon/cxxreact/Instance.h
index 547c971dfe9..6a8ba6f47dd 100644
--- a/packages/react-native/ReactCommon/cxxreact/Instance.h
+++ b/packages/react-native/ReactCommon/cxxreact/Instance.h
@@ -136,7 +136,7 @@ class RN_EXPORT Instance : private jsinspector_modern::InstanceTargetDelegate {
 
   /**
    * Unregisters the instance from the inspector. This method must be called
-   * on the main (non-JS) thread.
+   * on the main (non-JS) thread, AFTER \c initializeBridge has completed.
    */
   void unregisterFromInspector();