From 426868b6c27d9b625115c2a66915c5c7c752cf3f Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 29 Oct 2019 16:18:59 -0700 Subject: [PATCH] Add enable_nullify_catalyst_instance_on_destroy MC and gate setting mCatalystInstance to null in ReactContext Summary: Mostly for easing open-source migration and not making a backwards-incompatible change (yet), we'll set this to false by default. Every app can opt-in to this if wanted but it's not necessary. This change is part of experiments surrounding more-aggressive teardown for Fabric and Bridgeless mode. Changelog: [Internal] - This has the effect of (by default) disabling the previous diff which caused ReactContext teardown to always set mCatalystInstance to null. Now that is opt-in behavior and off by default, so it's not longer a breaking change. Reviewed By: mdvacca Differential Revision: D18207302 fbshipit-source-id: 7acfc894415e966f652c7049849eef79c440a135 --- .../java/com/facebook/react/bridge/ReactContext.java | 5 ++++- .../com/facebook/react/config/ReactFeatureFlags.java | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 0fb3a8ec794..b3db3af93d2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -21,6 +21,7 @@ import com.facebook.infer.annotation.ThreadConfined; import com.facebook.react.bridge.queue.MessageQueueThread; import com.facebook.react.bridge.queue.ReactQueueConfiguration; import com.facebook.react.common.LifecycleState; +import com.facebook.react.config.ReactFeatureFlags; import java.lang.ref.WeakReference; import java.util.concurrent.CopyOnWriteArraySet; @@ -272,7 +273,9 @@ public class ReactContext extends ContextWrapper { if (mCatalystInstance != null) { mCatalystInstance.destroy(); - mCatalystInstance = null; + if (ReactFeatureFlags.nullifyCatalystInstanceOnDestroy) { + mCatalystInstance = null; + } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 934994a62a6..b56aea77d85 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -61,4 +61,13 @@ public class ReactFeatureFlags { * CatalystInstanceImpl `destroy` method. */ public static boolean useCatalystTeardownV2 = false; + + /** + * When the ReactContext is destroyed, should the CatalystInstance immediately be nullified? This + * is the safest thing to do since the CatalystInstance shouldn't be used, and should be + * garbage-collected after it's destroyed, but this is a breaking change in that many native + * modules assume that a ReactContext will always have a CatalystInstance. This will be deleted + * and the CatalystInstance will always be destroyed in some future release. + */ + public static boolean nullifyCatalystInstanceOnDestroy = false; }