Skip to content

Commit

Permalink
Add enable_nullify_catalyst_instance_on_destroy MC and gate setting m…
Browse files Browse the repository at this point in the history
…CatalystInstance 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
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Oct 29, 2019
1 parent 55e974d commit 426868b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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;

Expand Down Expand Up @@ -272,7 +273,9 @@ public void destroy() {

if (mCatalystInstance != null) {
mCatalystInstance.destroy();
mCatalystInstance = null;
if (ReactFeatureFlags.nullifyCatalystInstanceOnDestroy) {
mCatalystInstance = null;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 426868b

Please sign in to comment.