Wait until everything is destroyed before returning from CatalystInstanceImpl.destroy

Summary:
CatalystInstanceImpl.destroy does a bunch of stuff in each of the relevant threads (Native Module thread, JS thread, UI thread).

This change creates a V1 destroy method (unchanged) and a V2 destroy method. The goal is to resolve (and catch!) race conditions in native modules and JSI modules that could occur during teardown; and mitigate race conditions that occur in RN teardown, like deallocation of C++ objects (scheduler, JS VM, and UIManager for Fabric).

Changelog:
[Internal] Experiment to fix deallocation race conditions

Reviewed By: mdvacca

Differential Revision: D18001677

fbshipit-source-id: 5955da0a7b726491c7d749642475f0fba74cce5a

ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

@@ -26,6 +26,7 @@ import com.facebook.react.bridge.queue.ReactQueueConfigurationImpl;
 import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.common.annotations.VisibleForTesting;
+import com.facebook.react.config.ReactFeatureFlags;
 import com.facebook.react.module.annotations.ReactModule;
 import com.facebook.react.turbomodule.core.CallInvokerHolderImpl;
 import com.facebook.react.turbomodule.core.interfaces.TurboModule;
@@ -85,6 +86,8 @@ public class CatalystInstanceImpl implements CatalystInstance {
   private final String mJsPendingCallsTitleForTrace =
       "pending_js_calls_instance" + sNextInstanceIdForTrace.getAndIncrement();
   private volatile boolean mDestroyed = false;
+  private volatile boolean mNativeModulesThreadDestructionComplete = false;
+  private volatile boolean mJSThreadDestructionComplete = false;
   private final TraceListener mTraceListener;
   private final JavaScriptModuleRegistry mJSModuleRegistry;
   private final JSBundleLoader mJSBundleLoader;
@@ -333,6 +336,18 @@ public class CatalystInstanceImpl implements CatalystInstance {
     Log.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() start");
     UiThreadUtil.assertOnUiThread();
 
+    if (ReactFeatureFlags.useCatalystTeardownV2) {
+      destroyV2();
+    } else {
+      destroyV1();
+    }
+  }
+
+  @ThreadConfined(UI)
+  public void destroyV1() {
+    Log.d(ReactConstants.TAG, "CatalystInstanceImpl.destroyV1() start");
+    UiThreadUtil.assertOnUiThread();
+
     if (mDestroyed) {
       return;
     }
@@ -408,6 +423,101 @@ public class CatalystInstanceImpl implements CatalystInstance {
     Systrace.unregisterListener(mTraceListener);
   }
 
+  /**
+   * Destroys this catalyst instance, waiting for any other threads in ReactQueueConfiguration
+   * (besides the UI thread) to finish running. Must be called from the UI thread so that we can
+   * fully shut down other threads.
+   */
+  @ThreadConfined(UI)
+  public void destroyV2() {
+    Log.d(ReactConstants.TAG, "CatalystInstanceImpl.destroyV2() start");
+    UiThreadUtil.assertOnUiThread();
+
+    if (mDestroyed) {
+      return;
+    }
+
+    // TODO: tell all APIs to shut down
+    ReactMarker.logMarker(ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_START);
+    mDestroyed = true;
+    mNativeModulesThreadDestructionComplete = false;
+    mJSThreadDestructionComplete = false;
+
+    mNativeModulesQueueThread.runOnQueue(
+        new Runnable() {
+          @Override
+          public void run() {
+            Log.d("CatalystInstanceImpl", ".destroy on native modules thread");
+            mNativeModuleRegistry.notifyJSInstanceDestroy();
+
+            // Notifies all JSI modules that they are being destroyed, including the FabricUIManager
+            // and Fabric Scheduler
+            mJSIModuleRegistry.notifyJSInstanceDestroy();
+            boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0);
+            if (!mBridgeIdleListeners.isEmpty()) {
+              for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) {
+                if (!wasIdle) {
+                  listener.onTransitionToBridgeIdle();
+                }
+                listener.onBridgeDestroyed();
+              }
+            }
+
+            mNativeModulesThreadDestructionComplete = true;
+            Log.d("CatalystInstanceImpl", ".destroy on native modules thread finished");
+          }
+        });
+
+    getReactQueueConfiguration()
+        .getJSQueueThread()
+        .runOnQueue(
+            new Runnable() {
+              @Override
+              public void run() {
+                Log.d("CatalystInstanceImpl", ".destroy on JS thread");
+                // We need to destroy the TurboModuleManager on the JS Thread
+                if (mTurboModuleManagerJSIModule != null) {
+                  mTurboModuleManagerJSIModule.onCatalystInstanceDestroy();
+                }
+
+                mJSThreadDestructionComplete = true;
+                Log.d("CatalystInstanceImpl", ".destroy on JS thread finished");
+              }
+            });
+
+    // Wait until destruction is complete
+    long waitStartTime = System.currentTimeMillis();
+    while (!mNativeModulesThreadDestructionComplete || !mJSThreadDestructionComplete) {
+      // Never wait here, blocking the UI thread, for more than 100ms
+      if ((System.currentTimeMillis() - waitStartTime) > 100) {
+        Log.w(
+            ReactConstants.TAG,
+            "CatalystInstanceImpl.destroy() timed out waiting for Native Modules and JS thread teardown");
+        break;
+      }
+    }
+
+    // Kill non-UI threads from neutral third party
+    // potentially expensive, so don't run on UI thread
+
+    // contextHolder is used as a lock to guard against
+    // other users of the JS VM having the VM destroyed
+    // underneath them, so notify them before we reset
+    // Native
+    mJavaScriptContextHolder.clear();
+
+    // Imperatively destruct the C++ CatalystInstance rather than
+    // wait for the JVM's GC to free it.
+    mHybridData.resetNative();
+
+    getReactQueueConfiguration().destroy();
+    Log.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() end");
+    ReactMarker.logMarker(ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_END);
+
+    // This is a noop if the listener was not yet registered.
+    Systrace.unregisterListener(mTraceListener);
+  }
+
   @Override
   public boolean isDestroyed() {
     return mDestroyed;

ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java

@@ -55,4 +55,10 @@ public class ReactFeatureFlags {
    * ...$$PropsSetter} class will be used instead.
    */
   public static boolean useViewManagerDelegates = false;
+
+  /**
+   * Should this application use Catalyst Teardown V2? This is an experiment to use a V2 of the
+   * CatalystInstanceImpl `destroy` method.
+   */
+  public static boolean useCatalystTeardownV2 = false;
 }