Skip to content

Commit

Permalink
Don't call jsQueue->quitSynchronous() immediately after construction (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
tomekzaw authored Jan 13, 2025
1 parent 082dc45 commit 15b3351
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ WorkletsModuleProxy::WorkletsModuleProxy(
valueUnpackerCode_)) {}

WorkletsModuleProxy::~WorkletsModuleProxy() {
jsQueue_->quitSynchronous();
uiWorkletRuntime_.reset();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ ReanimatedHermesRuntime::ReanimatedHermesRuntime(
auto adapter =
std::make_unique<HermesExecutorRuntimeAdapter>(*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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,17 @@ std::shared_ptr<jsi::Runtime> 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<ReanimatedHermesRuntime>(
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<rnv8::V8RuntimeConfig>();
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
}
Expand Down

0 comments on commit 15b3351

Please sign in to comment.