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 d35a599fbfa..8416fe580ff 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -23,6 +23,13 @@ public class ReactFeatureFlags { */ public static volatile boolean useTurboModules = false; + /** + * Should application use the new TM callback manager in Cxx? This is assumed to be a sane + * default, but it's new. We will delete once (1) we know it's safe to ship and (2) we have + * quantified impact. + */ + public static volatile boolean useTurboModulesRAIICallbackManager = false; + /** Should we dispatch TurboModule methods with promise returns to the NativeModules thread? */ public static volatile boolean enableTurboModulePromiseAsyncDispatch = 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 fe0399529bd..f70251b70bf 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,6 +16,7 @@ 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; @@ -58,7 +59,8 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { runtimeExecutor, (CallInvokerHolderImpl) jsCallInvokerHolder, (CallInvokerHolderImpl) nativeCallInvokerHolder, - delegate); + delegate, + ReactFeatureFlags.useTurboModulesRAIICallbackManager); installJSIBindings(); mEagerInitModuleNames = @@ -290,7 +292,8 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { RuntimeExecutor runtimeExecutor, CallInvokerHolderImpl jsCallInvokerHolder, CallInvokerHolderImpl nativeCallInvokerHolder, - TurboModuleManagerDelegate tmmDelegate); + TurboModuleManagerDelegate tmmDelegate, + boolean useTurboModulesRAIICallbackManager); private native void installJSIBindings(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp index 5878375bd36..2c567cf94d0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp @@ -39,10 +39,15 @@ jni::local_ref TurboModuleManager::initHybrid( jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, - jni::alias_ref delegate) { + jni::alias_ref delegate, + bool useTurboModulesRAIICallbackManager) { auto jsCallInvoker = jsCallInvokerHolder->cthis()->getCallInvoker(); auto nativeCallInvoker = nativeCallInvokerHolder->cthis()->getCallInvoker(); + if (useTurboModulesRAIICallbackManager) { + JavaTurboModule::enableUseTurboModulesRAIICallbackManager(true); + } + return makeCxxInstance( jThis, runtimeExecutor->cthis()->get(), diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h index a8b05fda4d5..15e0e7d094a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h @@ -32,7 +32,8 @@ class TurboModuleManager : public jni::HybridClass { jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, - jni::alias_ref delegate); + jni::alias_ref delegate, + bool useTurboModulesRAIICallbackManager); static void registerNatives(); private: diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h index 30add1d265b..8b52e0c2e8a 100644 --- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h +++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h @@ -84,5 +84,23 @@ class CallbackWrapper : public LongLivedObject { } }; +class RAIICallbackWrapperDestroyer { + public: + RAIICallbackWrapperDestroyer(std::weak_ptr callbackWrapper) + : callbackWrapper_(callbackWrapper) {} + + ~RAIICallbackWrapperDestroyer() { + auto strongWrapper = callbackWrapper_.lock(); + if (!strongWrapper) { + return; + } + + strongWrapper->destroy(); + } + + private: + std::weak_ptr callbackWrapper_; +}; + } // namespace react } // namespace facebook diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index a8bb21897ba..aa11f4579dc 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -52,6 +52,11 @@ JavaTurboModule::~JavaTurboModule() { }); } +bool JavaTurboModule::useTurboModulesRAIICallbackManager_ = false; +void JavaTurboModule::enableUseTurboModulesRAIICallbackManager(bool enable) { + JavaTurboModule::useTurboModulesRAIICallbackManager_ = enable; +} + namespace { jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, @@ -60,9 +65,20 @@ jni::local_ref createJavaCallbackFromJSIFunction( auto weakWrapper = react::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 + // not. + // 2. It cannot be weak_ptr since we need this object to live on. + // 3. It cannot be a value, because that would be deleted as soon as this + // function returns. + auto callbackWrapperOwner = + (JavaTurboModule::useTurboModulesRAIICallbackManager_ + ? std::make_shared(weakWrapper) + : nullptr); + std::function fn = - [weakWrapper, - wrapperWasCalled = false](folly::dynamic responses) mutable { + [weakWrapper, callbackWrapperOwner, wrapperWasCalled = false]( + folly::dynamic responses) mutable { if (wrapperWasCalled) { throw std::runtime_error( "callback 2 arg cannot be called more than once"); @@ -73,35 +89,41 @@ jni::local_ref createJavaCallbackFromJSIFunction( return; } - strongWrapper->jsInvoker().invokeAsync([weakWrapper, responses]() { - auto strongWrapper2 = weakWrapper.lock(); - if (!strongWrapper2) { - return; - } + strongWrapper->jsInvoker().invokeAsync( + [weakWrapper, callbackWrapperOwner, responses]() mutable { + auto strongWrapper2 = weakWrapper.lock(); + if (!strongWrapper2) { + return; + } - // TODO (T43155926) valueFromDynamic already returns a Value array. - // Don't iterate again - jsi::Value args = - jsi::valueFromDynamic(strongWrapper2->runtime(), responses); - auto argsArray = args.getObject(strongWrapper2->runtime()) - .asArray(strongWrapper2->runtime()); - std::vector result; - for (size_t i = 0; i < argsArray.size(strongWrapper2->runtime()); - i++) { - result.emplace_back( - strongWrapper2->runtime(), - argsArray.getValueAtIndex(strongWrapper2->runtime(), i)); - } - strongWrapper2->callback().call( - strongWrapper2->runtime(), - (const jsi::Value *)result.data(), - result.size()); + // TODO (T43155926) valueFromDynamic already returns a Value + // array. Don't iterate again + jsi::Value args = + jsi::valueFromDynamic(strongWrapper2->runtime(), responses); + auto argsArray = args.getObject(strongWrapper2->runtime()) + .asArray(strongWrapper2->runtime()); + std::vector result; + for (size_t i = 0; i < argsArray.size(strongWrapper2->runtime()); + i++) { + result.emplace_back( + strongWrapper2->runtime(), + argsArray.getValueAtIndex(strongWrapper2->runtime(), i)); + } + strongWrapper2->callback().call( + strongWrapper2->runtime(), + (const jsi::Value *)result.data(), + result.size()); - strongWrapper2->destroy(); - }); + if (JavaTurboModule::useTurboModulesRAIICallbackManager_) { + callbackWrapperOwner.reset(); + } else { + strongWrapper2->destroy(); + } + }); wrapperWasCalled = true; }; + return JCxxCallbackImpl::newObjectCxxArgs(fn); } diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h index 77a067701fc..4cdde8cb357 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h @@ -51,6 +51,13 @@ class JSI_EXPORT JavaTurboModule : public TurboModule { const jsi::Value *args, size_t argCount); + static void enableUseTurboModulesRAIICallbackManager(bool enable); + + /** + * Experiments + */ + static bool useTurboModulesRAIICallbackManager_; + private: jni::global_ref instance_; std::shared_ptr nativeInvoker_;