From bbf8a87ce75d96a95b632348b04099bd1b3ca39e Mon Sep 17 00:00:00 2001
From: Moti Zilberman <moti@meta.com>
Date: Thu, 6 Jun 2024 07:38:23 -0700
Subject: [PATCH] Defer HostTarget destruction until after the instance has
 been unregistered (#44767)

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

Changelog: [Internal]

Fixes a lifecycle bug in both the Bridge (`com.facebook.react.bridge`) and Bridgeless (`com.facebook.react.runtime`) integrations of Fusebox in React Native Android, whereby `HostTarget::unregisterInstance` gets called after the `HostTarget` has been destroyed.

The solution consists of two parts:

1. If a ReactHost / InstanceManager is asked to destroy itself while it contains no active ReactInstance / ReactContext, we destroy the `HostTarget` immediately.
2. Otherwise, if there *is* a live ReactInstance / ReactContext that has yet to be destroyed, we wait for that to happen before destroying the `HostTarget`. In practice, we do this by checking for the BEFORE_CREATE ( = Host destroyed) lifecycle state every time we destroy a ReactInstance / ReactContext.

Reviewed By: javache

Differential Revision: D58031215

fbshipit-source-id: 321c73e85afd17a1b38c63f73aee5ebb59c00686
---
 .../com/facebook/react/ReactInstanceManager.java | 15 ++++++++++++---
 .../facebook/react/runtime/ReactHostImpl.java    | 16 ++++++++++++++--
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
index ea77bef5731..41c3e9f086e 100644
--- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
+++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
@@ -701,7 +701,6 @@ public class ReactInstanceManager {
   @Deprecated
   public void onHostDestroy() {
     UiThreadUtil.assertOnUiThread();
-    destroyInspectorTarget();
 
     if (mUseDeveloperSupport) {
       mDevSupportManager.setDevSupportEnabled(false);
@@ -752,7 +751,6 @@ public class ReactInstanceManager {
     }
 
     mHasStartedDestroying = true;
-    destroyInspectorTarget();
 
     if (mUseDeveloperSupport) {
       mDevSupportManager.setDevSupportEnabled(false);
@@ -782,6 +780,12 @@ public class ReactInstanceManager {
       }
     }
 
+    // If the host is being destroyed, now that the current context/instance
+    // has been destroyed, we can safely destroy the host's inspector target.
+    if (mLifecycleState == LifecycleState.BEFORE_CREATE) {
+      destroyInspectorHostTarget();
+    }
+
     mHasStartedCreatingInitialContext = false;
     if (!mKeepActivity) {
       mCurrentActivity = null;
@@ -835,6 +839,10 @@ public class ReactInstanceManager {
       if (mLifecycleState == LifecycleState.BEFORE_RESUME) {
         currentContext.onHostDestroy(mKeepActivity);
       }
+    } else {
+      // There's no current context that requires the host inspector target to
+      // be kept alive, so we can destroy it immediately.
+      destroyInspectorHostTarget();
     }
     mLifecycleState = LifecycleState.BEFORE_CREATE;
   }
@@ -1542,7 +1550,8 @@ public class ReactInstanceManager {
     return mInspectorTarget;
   }
 
-  private void destroyInspectorTarget() {
+  @ThreadConfined(UI)
+  private void destroyInspectorHostTarget() {
     if (mInspectorTarget != null) {
       mInspectorTarget.close();
       mInspectorTarget = null;
diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
index e58483008be..d37e4ffde1c 100644
--- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
+++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
@@ -887,7 +887,11 @@ public class ReactHostImpl implements ReactHost {
   @ThreadConfined(UI)
   private void moveToHostDestroy(@Nullable ReactContext currentContext) {
     mReactLifecycleStateManager.moveToOnHostDestroy(currentContext);
-    destroyReactHostInspectorTarget();
+    if (currentContext == null) {
+      // There's no current context/instance that requires the host inspector
+      // target to be kept alive, so we can destroy it immediately.
+      destroyInspectorHostTarget();
+    }
     setCurrentActivity(null);
   }
 
@@ -1664,13 +1668,15 @@ public class ReactHostImpl implements ReactHost {
     return mReactHostInspectorTarget;
   }
 
-  private void destroyReactHostInspectorTarget() {
+  @ThreadConfined(UI)
+  private void destroyInspectorHostTarget() {
     if (mReactHostInspectorTarget != null) {
       mReactHostInspectorTarget.close();
       mReactHostInspectorTarget = null;
     }
   }
 
+  @ThreadConfined(UI)
   private void unregisterInstanceFromInspector(final @Nullable ReactInstance reactInstance) {
     if (reactInstance != null) {
       if (InspectorFlags.getFuseboxEnabled()) {
@@ -1680,5 +1686,11 @@ public class ReactHostImpl implements ReactHost {
       }
       reactInstance.unregisterFromInspector();
     }
+    if (mReactLifecycleStateManager.getLifecycleState() == LifecycleState.BEFORE_CREATE) {
+      // If the host is being destroyed, now that the current context/instance
+      // has been unregistered, we can safely destroy the host's inspector
+      // target.
+      destroyInspectorHostTarget();
+    }
   }
 }