Remove flags around Catalyst teardown

Summary:
These flags haven't been used in months. They were useful to uncover some race conditions, but will not be iterated further. The Venice project will obviate the concerns that sparked these experiments in the first place.

These flags have been hardcoded to false for a while.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22319204

fbshipit-source-id: 09415f3bb1ca56e15f357210e966d0483ff384f2
This commit is contained in:
Joshua Gross
2020-06-30 16:12:05 -07:00
committed by Facebook GitHub Bot
parent 85f1e984bb
commit f493a316fd
3 changed files with 1 additions and 124 deletions
@@ -340,16 +340,6 @@ public class CatalystInstanceImpl implements CatalystInstance {
FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() start");
UiThreadUtil.assertOnUiThread();
if (ReactFeatureFlags.useCatalystTeardownV2) {
destroyV2();
} else {
destroyV1();
}
}
@ThreadConfined(UI)
public void destroyV1() {
FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroyV1() start");
UiThreadUtil.assertOnUiThread();
if (mDestroyed) {
@@ -427,101 +417,6 @@ 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() {
FLog.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() {
FLog.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;
FLog.d("CatalystInstanceImpl", ".destroy on native modules thread finished");
}
});
getReactQueueConfiguration()
.getJSQueueThread()
.runOnQueue(
new Runnable() {
@Override
public void run() {
FLog.d("CatalystInstanceImpl", ".destroy on JS thread");
// We need to destroy the TurboModuleManager on the JS Thread
if (mTurboModuleManagerJSIModule != null) {
mTurboModuleManagerJSIModule.onCatalystInstanceDestroy();
}
mJSThreadDestructionComplete = true;
FLog.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) {
FLog.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();
FLog.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;
@@ -296,9 +296,6 @@ public class ReactContext extends ContextWrapper {
mDestroyed = true;
if (mCatalystInstance != null) {
mCatalystInstance.destroy();
if (ReactFeatureFlags.nullifyCatalystInstanceOnDestroy) {
mCatalystInstance = null;
}
}
}
@@ -41,22 +41,7 @@ public class ReactFeatureFlags {
* inside view manager will be called instead.
*/
public static boolean useViewManagerDelegatesForCommands = 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;
/**
* 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;
/**
* This react flag enables a custom algorithm for the getChildVisibleRect() method in the classes
* ReactViewGroup, ReactHorizontalScrollView and ReactScrollView.