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 0b5c2402428..062659dcabe 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -55,26 +55,6 @@ public class ReactFeatureFlags { /** Should we dispatch TurboModule methods with promise returns to the NativeModules thread? */ public static volatile boolean enableTurboModulePromiseAsyncDispatch = false; - /** - * Experiment: - * - *

Bridge and Bridgeless mode can run concurrently. This means that there can be two - * TurboModule systems alive at the same time. - * - *

The TurboModule system stores all JS callbacks in a global LongLivedObjectCollection. This - * collection is cleared when the JS VM is torn down. Implication: Tearing down the bridge JSVM - * invalidates the bridgeless JSVM's callbacks, and vice versa. - * - *

useGlobalCallbackCleanupScopeUsingRetainJSCallback => Use a retainJSCallbacks lambda to - * store jsi::Functions into the global LongLivedObjectCollection - * - *

useTurboModuleManagerCallbackCleanupScope => Use a retainJSCallbacks labmda to store - * jsi::Functions into a LongLivedObjectCollection owned by the TurboModuleManager - */ - public static boolean useGlobalCallbackCleanupScopeUsingRetainJSCallback = false; - - public static boolean useTurboModuleManagerCallbackCleanupScope = false; - /** This feature flag enables logs for Fabric */ public static boolean enableFabricLogs = false; diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java index 440037ab7cd..31c3c05b677 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java @@ -16,7 +16,6 @@ import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.bridge.CxxModuleWrapper; import com.facebook.react.bridge.JSIModule; import com.facebook.react.bridge.RuntimeExecutor; -import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder; import com.facebook.react.turbomodule.core.interfaces.TurboModule; import com.facebook.react.turbomodule.core.interfaces.TurboModuleRegistry; @@ -59,9 +58,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { runtimeExecutor, (CallInvokerHolderImpl) jsCallInvokerHolder, (CallInvokerHolderImpl) nativeCallInvokerHolder, - delegate, - ReactFeatureFlags.useGlobalCallbackCleanupScopeUsingRetainJSCallback, - ReactFeatureFlags.useTurboModuleManagerCallbackCleanupScope); + delegate); installJSIBindings(); mEagerInitModuleNames = @@ -293,9 +290,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { RuntimeExecutor runtimeExecutor, CallInvokerHolderImpl jsCallInvokerHolder, CallInvokerHolderImpl nativeCallInvokerHolder, - TurboModuleManagerDelegate tmmDelegate, - boolean useGlobalCallbackCleanupScopeUsingRetainJSCallback, - boolean useTurboModuleManagerCallbackCleanupScope); + TurboModuleManagerDelegate tmmDelegate); private native void installJSIBindings(); diff --git a/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.cpp b/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.cpp index a4d1c1289d9..31f437bc3cf 100644 --- a/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.cpp +++ b/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.cpp @@ -36,55 +36,26 @@ TurboModuleManager::TurboModuleManager( RuntimeExecutor runtimeExecutor, std::shared_ptr jsCallInvoker, std::shared_ptr nativeCallInvoker, - jni::alias_ref delegate, - bool useGlobalCallbackCleanupScopeUsingRetainJSCallback, - bool useTurboModuleManagerCallbackCleanupScope) + jni::alias_ref delegate) : javaPart_(jni::make_global(jThis)), runtimeExecutor_(runtimeExecutor), jsCallInvoker_(jsCallInvoker), nativeCallInvoker_(nativeCallInvoker), delegate_(jni::make_global(delegate)), - turboModuleCache_(std::make_shared()) { - if (useGlobalCallbackCleanupScopeUsingRetainJSCallback) { - longLivedObjectCollection_ = nullptr; - retainJSCallback_ = [](jsi::Function &&callback, - jsi::Runtime &runtime, - std::shared_ptr jsInvoker) { - return CallbackWrapper::createWeak( - std::move(callback), runtime, jsInvoker); - }; - } else if (useTurboModuleManagerCallbackCleanupScope) { - longLivedObjectCollection_ = std::make_shared(); - retainJSCallback_ = [longLivedObjectCollection = - longLivedObjectCollection_]( - jsi::Function &&callback, - jsi::Runtime &runtime, - std::shared_ptr jsInvoker) { - return CallbackWrapper::createWeak( - longLivedObjectCollection, std::move(callback), runtime, jsInvoker); - }; - } -} + turboModuleCache_(std::make_shared()) {} jni::local_ref TurboModuleManager::initHybrid( jni::alias_ref jThis, jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, - jni::alias_ref delegate, - bool useGlobalCallbackCleanupScopeUsingRetainJSCallback, - bool useTurboModuleManagerCallbackCleanupScope) { - auto jsCallInvoker = jsCallInvokerHolder->cthis()->getCallInvoker(); - auto nativeCallInvoker = nativeCallInvokerHolder->cthis()->getCallInvoker(); - + jni::alias_ref delegate) { return makeCxxInstance( jThis, runtimeExecutor->cthis()->get(), - jsCallInvoker, - nativeCallInvoker, - delegate, - useGlobalCallbackCleanupScopeUsingRetainJSCallback, - useTurboModuleManagerCallbackCleanupScope); + jsCallInvokerHolder->cthis()->getCallInvoker(), + nativeCallInvokerHolder->cthis()->getCallInvoker(), + delegate); } void TurboModuleManager::registerNatives() { @@ -106,8 +77,7 @@ void TurboModuleManager::installJSIBindings() { jsCallInvoker_ = std::weak_ptr(jsCallInvoker_), nativeCallInvoker_ = std::weak_ptr(nativeCallInvoker_), delegate_ = jni::make_weak(delegate_), - javaPart_ = jni::make_weak(javaPart_), - retainJSCallback = retainJSCallback_]( + javaPart_ = jni::make_weak(javaPart_)]( const std::string &name) -> std::shared_ptr { auto turboModuleCache = turboModuleCache_.lock(); auto jsCallInvoker = jsCallInvoker_.lock(); @@ -168,8 +138,7 @@ void TurboModuleManager::installJSIBindings() { .moduleName = name, .instance = moduleInstance, .jsInvoker = jsCallInvoker, - .nativeInvoker = nativeCallInvoker, - .retainJSCallback = retainJSCallback}; + .nativeInvoker = nativeCallInvoker}; auto turboModule = delegate->cthis()->getTurboModule(name, params); turboModuleCache->insert({name, turboModule}); @@ -183,10 +152,7 @@ void TurboModuleManager::installJSIBindings() { TurboModuleBindingMode bindingMode = static_cast( getFeatureFlagValue("turboModuleBindingMode")); TurboModuleBinding::install( - runtime, - std::move(turboModuleProvider), - bindingMode, - longLivedObjectCollection_); + runtime, std::move(turboModuleProvider), bindingMode, nullptr); }); } diff --git a/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.h b/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.h index 26c13fd5506..98bbc704c8f 100644 --- a/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.h +++ b/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.h @@ -32,9 +32,7 @@ class TurboModuleManager : public jni::HybridClass { jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, - jni::alias_ref delegate, - bool useGlobalCallbackCleanupScopeUsingRetainJSCallback, - bool useTurboModuleManagerCallbackCleanupScope); + jni::alias_ref delegate); static void registerNatives(); private: @@ -45,9 +43,6 @@ class TurboModuleManager : public jni::HybridClass { std::shared_ptr nativeCallInvoker_; jni::global_ref delegate_; - JSCallbackRetainer retainJSCallback_; - std::shared_ptr longLivedObjectCollection_; - using TurboModuleCache = std::unordered_map>; @@ -65,9 +60,7 @@ class TurboModuleManager : public jni::HybridClass { RuntimeExecutor runtimeExecutor, std::shared_ptr jsCallInvoker, std::shared_ptr nativeCallInvoker, - jni::alias_ref delegate, - bool useGlobalCallbackCleanupScopeUsingRetainJSCallback, - bool useTurboModuleManagerCallbackCleanupScope); + jni::alias_ref delegate); }; } // namespace react diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 92e4c2bceb4..3240769c64a 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -31,8 +31,7 @@ namespace TMPL = TurboModulePerfLogger; JavaTurboModule::JavaTurboModule(const InitParams ¶ms) : TurboModule(params.moduleName, params.jsInvoker), instance_(jni::make_global(params.instance)), - nativeInvoker_(params.nativeInvoker), - retainJSCallback_(params.retainJSCallback) {} + nativeInvoker_(params.nativeInvoker) {} JavaTurboModule::~JavaTurboModule() { /** @@ -63,13 +62,11 @@ struct JNIArgs { }; jni::local_ref createJavaCallbackFromJSIFunction( - const JSCallbackRetainer &retainJSCallback, jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker) { - auto weakWrapper = retainJSCallback != nullptr - ? retainJSCallback(std::move(function), rt, jsInvoker) - : react::CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); + auto weakWrapper = + CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); // This needs to be a shared_ptr because: // 1. It cannot be unique_ptr. std::function is copyable but unique_ptr is @@ -253,8 +250,7 @@ JNIArgs convertJSIArgsToJNIArgs( const jsi::Value *args, size_t count, const std::shared_ptr &jsInvoker, - TurboModuleMethodValueKind valueKind, - const JSCallbackRetainer &retainJSCallback) { + TurboModuleMethodValueKind valueKind) { unsigned int expectedArgumentCount = valueKind == PromiseKind ? methodArgTypes.size() - 1 : methodArgTypes.size(); @@ -334,8 +330,7 @@ JNIArgs convertJSIArgsToJNIArgs( } jsi::Function fn = arg->getObject(rt).getFunction(rt); jarg->l = makeGlobalIfNecessary( - createJavaCallbackFromJSIFunction( - retainJSCallback, std::move(fn), rt, jsInvoker) + createJavaCallbackFromJSIFunction(std::move(fn), rt, jsInvoker) .release()); } else if (type == "Lcom/facebook/react/bridge/ReadableArray;") { if (!(arg->isObject() && arg->getObject(rt).isArray(rt))) { @@ -492,8 +487,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( args, argCount, jsInvoker_, - valueKind, - retainJSCallback_); + valueKind); if (isMethodSync && valueKind != PromiseKind) { TMPL::syncMethodCallArgConversionEnd(moduleName, methodName); @@ -696,8 +690,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( methodID, moduleNameStr = name_, methodNameStr, - env, - retainJSCallback = retainJSCallback_]( + env]( jsi::Runtime &runtime, const jsi::Value &thisVal, const jsi::Value *promiseConstructorArgs, @@ -714,16 +707,10 @@ jsi::Value JavaTurboModule::invokeJavaMethod( runtime); auto resolve = createJavaCallbackFromJSIFunction( - retainJSCallback, - std::move(resolveJSIFn), - runtime, - jsInvoker_) + std::move(resolveJSIFn), runtime, jsInvoker_) .release(); auto reject = createJavaCallbackFromJSIFunction( - retainJSCallback, - std::move(rejectJSIFn), - runtime, - jsInvoker_) + std::move(rejectJSIFn), runtime, jsInvoker_) .release(); jclass jPromiseImpl = diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h index e420e46d147..3581fedea21 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h @@ -25,11 +25,6 @@ struct JTurboModule : jni::JavaClass { "Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;"; }; -using JSCallbackRetainer = std::function( - jsi::Function &&callback, - jsi::Runtime &runtime, - const std::shared_ptr &jsInvoker)>; - class JSI_EXPORT JavaTurboModule : public TurboModule { public: // TODO(T65603471): Should we unify this with a Fabric abstraction? @@ -38,7 +33,6 @@ class JSI_EXPORT JavaTurboModule : public TurboModule { jni::alias_ref instance; std::shared_ptr jsInvoker; std::shared_ptr nativeInvoker; - JSCallbackRetainer retainJSCallback; }; JavaTurboModule(const InitParams ¶ms); @@ -56,7 +50,6 @@ class JSI_EXPORT JavaTurboModule : public TurboModule { private: jni::global_ref instance_; std::shared_ptr nativeInvoker_; - JSCallbackRetainer retainJSCallback_; }; } // namespace react