From 15b33519255eaabdea1794e09b80142851157757 Mon Sep 17 00:00:00 2001 From: Tomek Zawadzki Date: Mon, 13 Jan 2025 13:03:31 +0100 Subject: [PATCH] Don't call `jsQueue->quitSynchronous()` immediately after construction (#6781) ## Summary This PR moves `jsQueue->quitSynchronously()` calls from `ReanimatedRuntime::make` or `ReanimatedHermesRuntime` constructor to `~WorkletsModuleProxy` as well as simplifies the logic. I had to add `(void)jsQueue;` to avoid a warning due to unused variable. ## Test plan > [!NOTE] > I have tested this PR only on iOS Fabric in debug mode. Please test it thoroughly across platforms, runtimes and build configurations prior to merging. --- .../worklets/NativeModules/WorkletsModuleProxy.cpp | 1 + .../WorkletRuntime/ReanimatedHermesRuntime.cpp | 4 ---- .../worklets/WorkletRuntime/ReanimatedRuntime.cpp | 13 ++----------- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/react-native-reanimated/Common/cpp/worklets/NativeModules/WorkletsModuleProxy.cpp b/packages/react-native-reanimated/Common/cpp/worklets/NativeModules/WorkletsModuleProxy.cpp index 3c32a1fdb21..69635867ac5 100644 --- a/packages/react-native-reanimated/Common/cpp/worklets/NativeModules/WorkletsModuleProxy.cpp +++ b/packages/react-native-reanimated/Common/cpp/worklets/NativeModules/WorkletsModuleProxy.cpp @@ -52,6 +52,7 @@ WorkletsModuleProxy::WorkletsModuleProxy( valueUnpackerCode_)) {} WorkletsModuleProxy::~WorkletsModuleProxy() { + jsQueue_->quitSynchronous(); uiWorkletRuntime_.reset(); } diff --git a/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedHermesRuntime.cpp b/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedHermesRuntime.cpp index 29f068c9e4c..d32241dd607 100644 --- a/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedHermesRuntime.cpp +++ b/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedHermesRuntime.cpp @@ -64,10 +64,6 @@ ReanimatedHermesRuntime::ReanimatedHermesRuntime( auto adapter = std::make_unique(*runtime_, jsQueue); debugToken_ = chrome::enableDebugging(std::move(adapter), name); -#else - // This is required by iOS, because there is an assertion in the destructor - // that the thread was indeed `quit` before - jsQueue->quitSynchronous(); #endif // HERMES_ENABLE_DEBUGGER #ifndef NDEBUG diff --git a/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedRuntime.cpp b/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedRuntime.cpp index fff5f6f37e7..44a024242ff 100644 --- a/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedRuntime.cpp +++ b/packages/react-native-reanimated/Common/cpp/worklets/WorkletRuntime/ReanimatedRuntime.cpp @@ -26,26 +26,17 @@ std::shared_ptr ReanimatedRuntime::make( const std::string &name) { (void)rnRuntime; // used only for V8 #if JS_RUNTIME_HERMES - // We don't call `jsQueue->quitSynchronous()` here, since it will be done - // later in ReanimatedHermesRuntime - auto runtime = facebook::hermes::makeHermesRuntime(); return std::make_shared( std::move(runtime), jsQueue, name); #elif JS_RUNTIME_V8 - // This is required by iOS, because there is an assertion in the destructor - // that the thread was indeed `quit` before. - jsQueue->quitSynchronous(); - + (void)jsQueue; auto config = std::make_unique(); config->enableInspector = false; config->appName = name; return rnv8::createSharedV8Runtime(&rnRuntime, std::move(config)); #else - // This is required by iOS, because there is an assertion in the destructor - // that the thread was indeed `quit` before - jsQueue->quitSynchronous(); - + (void)jsQueue; return facebook::jsc::makeJSCRuntime(); #endif }