Skip to content

Commit

Permalink
fix[Runtime.evaluate]: registered RuntimeExecutor should go through R…
Browse files Browse the repository at this point in the history
…untimeScheduler (#47119)

Summary:
Pull Request resolved: #47119

# Changelog:
[General] [Fixed] - Microtasks are now correctly executed after the code evaluation in Console panel of DevTools.

Fixes #46966.

`runtimeExecutor` which is propagated here, is actually being used by Hermes:
https://www.internalfb.com/code/fbsource/[cba75f2b515a]/xplat/js/react-native-github/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp?lines=112-113

The issue was that any expression that should be evaluated as part of `Runtime.evaluate` was not going through `RuntimeScheduler`, because specified `runtimeExecutor` was not going through it as well, and it was defined prior to `RuntimeScheduler`. Because of this, `RuntimeScheduler` was not draining out the microtasks queue and basically any scheduled Microtasks were not executed, see T200616136.

With this fix, we create an executor that goes through `RuntimeScheduler`, which is using another executor that makes sure that all scheduled callbacks are only executed after `Inspector` was setup.

It is extremely messy and in the future we should untangle these circular dependencies and try to simplify the approach.

Reviewed By: rubennorte

Differential Revision: D64552372

fbshipit-source-id: 467d37c71a2eb7c940297ce90ca8d68268b1ff33
  • Loading branch information
hoxyq authored and facebook-github-bot committed Oct 22, 2024
1 parent cc23aea commit 3dfe22b
Showing 1 changed file with 51 additions and 25 deletions.
76 changes: 51 additions & 25 deletions packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@

namespace facebook::react {

namespace {

std::shared_ptr<RuntimeScheduler> createRuntimeScheduler(
RuntimeExecutor runtimeExecutor,
RuntimeSchedulerTaskErrorHandler taskErrorHandler) {
std::shared_ptr<RuntimeScheduler> scheduler =
std::make_shared<RuntimeScheduler>(
runtimeExecutor, RuntimeSchedulerClock::now, taskErrorHandler);
scheduler->setPerformanceEntryReporter(
// FIXME: Move creation of PerformanceEntryReporter to here and
// guarantee that its lifetime is the same as the runtime.
PerformanceEntryReporter::getInstance().get());

return scheduler;
}

} // namespace

ReactInstance::ReactInstance(
std::unique_ptr<JSRuntime> runtime,
std::shared_ptr<MessageQueueThread> jsMessageQueueThread,
Expand Down Expand Up @@ -93,13 +111,34 @@ ReactInstance::ReactInstance(
if (parentInspectorTarget_) {
auto executor = parentInspectorTarget_->executorFromThis();

auto runtimeExecutorThatWaitsForInspectorSetup =
auto bufferedRuntimeExecutorThatWaitsForInspectorSetup =
std::make_shared<BufferedRuntimeExecutor>(runtimeExecutor);
auto runtimeExecutorThatExecutesAfterInspectorSetup =
[bufferedRuntimeExecutorThatWaitsForInspectorSetup](
std::function<void(jsi::Runtime & runtime)>&& callback) {
bufferedRuntimeExecutorThatWaitsForInspectorSetup->execute(
std::move(callback));
};

runtimeScheduler_ = createRuntimeScheduler(
runtimeExecutorThatExecutesAfterInspectorSetup,
[jsErrorHandler = jsErrorHandler_](
jsi::Runtime& runtime, jsi::JSError& error) {
jsErrorHandler->handleError(runtime, error, true);
});

auto runtimeExecutorThatGoesThroughRuntimeScheduler =
[runtimeScheduler = runtimeScheduler_.get()](
std::function<void(jsi::Runtime & runtime)>&& callback) {
runtimeScheduler->scheduleWork(std::move(callback));
};

// This code can execute from any thread, so we need to make sure we set up
// the inspector logic in the right one. The callback executes immediately
// if we are already in the right thread.
executor([this, runtimeExecutor, runtimeExecutorThatWaitsForInspectorSetup](
executor([this,
runtimeExecutorThatGoesThroughRuntimeScheduler,
bufferedRuntimeExecutorThatWaitsForInspectorSetup](
jsinspector_modern::HostTarget& hostTarget) {
// Callbacks scheduled through the page target executor are generally
// not guaranteed to run (e.g.: if the page target is destroyed)
Expand All @@ -110,32 +149,19 @@ ReactInstance::ReactInstance(
// creation task to finish before starting the destruction.
inspectorTarget_ = &hostTarget.registerInstance(*this);
runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
runtime_->getRuntimeTargetDelegate(), runtimeExecutor);
runtimeExecutorThatWaitsForInspectorSetup->flush();
runtime_->getRuntimeTargetDelegate(),
runtimeExecutorThatGoesThroughRuntimeScheduler);
bufferedRuntimeExecutorThatWaitsForInspectorSetup->flush();
});

// We decorate the runtime executor used everywhere else to wait for the
// inspector to finish its setup.
runtimeExecutor =
[runtimeExecutorThatWaitsForInspectorSetup](
std::function<void(jsi::Runtime & runtime)>&& callback) {
runtimeExecutorThatWaitsForInspectorSetup->execute(
std::move(callback));
};
} else {
runtimeScheduler_ = createRuntimeScheduler(
runtimeExecutor,
[jsErrorHandler = jsErrorHandler_](
jsi::Runtime& runtime, jsi::JSError& error) {
jsErrorHandler->handleError(runtime, error, true);
});
}

runtimeScheduler_ = std::make_shared<RuntimeScheduler>(
runtimeExecutor,
RuntimeSchedulerClock::now,
[jsErrorHandler = jsErrorHandler_](
jsi::Runtime& runtime, jsi::JSError& error) {
jsErrorHandler->handleError(runtime, error, true);
});
runtimeScheduler_->setPerformanceEntryReporter(
// FIXME: Move creation of PerformanceEntryReporter to here and guarantee
// that its lifetime is the same as the runtime.
PerformanceEntryReporter::getInstance().get());

bufferedRuntimeExecutor_ = std::make_shared<BufferedRuntimeExecutor>(
[runtimeScheduler = runtimeScheduler_.get()](
std::function<void(jsi::Runtime & runtime)>&& callback) {
Expand Down

0 comments on commit 3dfe22b

Please sign in to comment.