Skip to content

Commit

Permalink
Do not hold raw RuntimeScheduler pointer in BufferedRuntimeExecutor (#…
Browse files Browse the repository at this point in the history
…46542)

Summary:
Pull Request resolved: #46542

If `bufferedRuntimeExecutor_` is referenced beyond the lifetime of the ReactInstance, it may point to invalid memory. RuntimeScheduler already holds weak references to the actual runtime, so it's safe to retain that instead.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D62748768
  • Loading branch information
javache authored and facebook-github-bot committed Sep 18, 2024
1 parent e30de3a commit 6200771
Showing 1 changed file with 12 additions and 16 deletions.
28 changes: 12 additions & 16 deletions packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ ReactInstance::ReactInstance(
PerformanceEntryReporter::getInstance().get());

bufferedRuntimeExecutor_ = std::make_shared<BufferedRuntimeExecutor>(
[runtimeScheduler = runtimeScheduler_.get()](
[runtimeScheduler = runtimeScheduler_](
std::function<void(jsi::Runtime & runtime)>&& callback) {
runtimeScheduler->scheduleWork(std::move(callback));
});
Expand All @@ -156,7 +156,7 @@ void ReactInstance::unregisterFromInspector() {
}

RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {
return [runtimeScheduler = runtimeScheduler_.get()](
return [runtimeScheduler = runtimeScheduler_](
std::function<void(jsi::Runtime & runtime)>&& callback) {
runtimeScheduler->scheduleWork(std::move(callback));
};
Expand All @@ -167,12 +167,14 @@ RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {
// getUnbufferedRuntimeExecutor() instead if you do not need the main JS bundle
// to have finished. e.g. setting global variables into JS runtime.
RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept {
return [weakBufferedRuntimeExecutor_ =
// FIXME: we don't really need a weak reference here, bufferedRuntimeExecutor
// retains a strong reference to runtimeScheduler, which in turns retains weak
// references to the runtime
return [weakBufferedRuntimeExecutor =
std::weak_ptr<BufferedRuntimeExecutor>(bufferedRuntimeExecutor_)](
std::function<void(jsi::Runtime & runtime)>&& callback) {
if (auto strongBufferedRuntimeExecutor_ =
weakBufferedRuntimeExecutor_.lock()) {
strongBufferedRuntimeExecutor_->execute(std::move(callback));
if (auto bufferedRuntimeExecutor = weakBufferedRuntimeExecutor.lock()) {
bufferedRuntimeExecutor->execute(std::move(callback));
}
};
}
Expand Down Expand Up @@ -209,12 +211,8 @@ void ReactInstance::loadScript(
std::string scriptName = simpleBasename(sourceURL);

runtimeScheduler_->scheduleWork(
[this,
scriptName,
sourceURL,
buffer = std::move(buffer),
weakBufferedRuntimeExecuter = std::weak_ptr<BufferedRuntimeExecutor>(
bufferedRuntimeExecutor_)](jsi::Runtime& runtime) {
[this, scriptName, sourceURL, buffer = std::move(buffer)](
jsi::Runtime& runtime) {
SystraceSection s("ReactInstance::loadScript");
bool hasLogger(ReactMarker::logTaggedMarkerBridgelessImpl);
if (hasLogger) {
Expand All @@ -240,10 +238,8 @@ void ReactInstance::loadScript(
ReactMarker::INIT_REACT_RUNTIME_STOP);
ReactMarker::logMarkerBridgeless(ReactMarker::APP_STARTUP_STOP);
}
if (auto strongBufferedRuntimeExecuter =
weakBufferedRuntimeExecuter.lock()) {
strongBufferedRuntimeExecuter->flush();
}

bufferedRuntimeExecutor_->flush();
});
}

Expand Down

0 comments on commit 6200771

Please sign in to comment.