Skip to content

Commit afc033d

Browse files
javachefacebook-github-bot
authored andcommitted
Fix unsafe JSI access in UIManagerBinding::startViewTransition onReadyCallback
Summary: onReadyCallback captured jsi::Runtime& by reference but could be called from a non-JS thread by the view transition delegate. Route it through runtimeExecutor_ (matching the existing onCompleteCallback pattern). Document the threading contract on UIManagerViewTransitionDelegate::startViewTransition. Changelog: [Internal] Differential Revision: D98717252
1 parent b60f0c9 commit afc033d

File tree

2 files changed

+31
-19
lines changed

2 files changed

+31
-19
lines changed

packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,10 +1064,8 @@ jsi::Value UIManagerBinding::get(
10641064
auto promiseConstructor =
10651065
runtime.global().getPropertyAsFunction(runtime, "Promise");
10661066

1067-
auto readyResolveFunc =
1068-
std::make_shared<std::shared_ptr<jsi::Function>>();
1069-
auto finishedResolveFunc =
1070-
std::make_shared<std::shared_ptr<jsi::Function>>();
1067+
std::shared_ptr<jsi::Function> readyResolveFunc;
1068+
std::shared_ptr<jsi::Function> finishedResolveFunc;
10711069

10721070
auto mutationFunc = std::make_shared<jsi::Function>(
10731071
arguments[0].asObject(runtime).asFunction(runtime));
@@ -1078,14 +1076,13 @@ jsi::Value UIManagerBinding::get(
10781076
runtime,
10791077
jsi::PropNameID::forAscii(runtime, "readyExecutor"),
10801078
2,
1081-
[readyResolveFunc](
1079+
[&readyResolveFunc](
10821080
jsi::Runtime& runtime,
10831081
const jsi::Value& /*thisValue*/,
10841082
const jsi::Value* args,
10851083
size_t /*count*/) -> jsi::Value {
1086-
auto onReadyFunc = std::make_shared<jsi::Function>(
1084+
readyResolveFunc = std::make_shared<jsi::Function>(
10871085
args[0].asObject(runtime).asFunction(runtime));
1088-
*readyResolveFunc = onReadyFunc;
10891086
return jsi::Value::undefined();
10901087
}));
10911088

@@ -1095,19 +1092,21 @@ jsi::Value UIManagerBinding::get(
10951092
runtime,
10961093
jsi::PropNameID::forAscii(runtime, "finishedExecutor"),
10971094
2,
1098-
[finishedResolveFunc, viewTransitionDelegate](
1095+
[&finishedResolveFunc, viewTransitionDelegate](
10991096
jsi::Runtime& rt,
11001097
const jsi::Value& /*thisValue*/,
11011098
const jsi::Value* args,
1102-
size_t /*count*/) -> jsi::Value {
1099+
size_t /*count*/) mutable -> jsi::Value {
11031100
auto onCompleteFunc = std::make_shared<jsi::Function>(
11041101
args[0].asObject(rt).asFunction(rt));
1105-
*finishedResolveFunc = std::make_shared<jsi::Function>(
1102+
finishedResolveFunc = std::make_shared<jsi::Function>(
11061103
jsi::Function::createFromHostFunction(
11071104
rt,
11081105
jsi::PropNameID::forAscii(rt, "finishedResolve"),
11091106
0,
1110-
[onCompleteFunc, viewTransitionDelegate](
1107+
[onCompleteFunc = std::move(onCompleteFunc),
1108+
viewTransitionDelegate =
1109+
std::move(viewTransitionDelegate)](
11111110
jsi::Runtime& runtime,
11121111
const jsi::Value& /*thisValue*/,
11131112
const jsi::Value* /*args*/,
@@ -1125,20 +1124,28 @@ jsi::Value UIManagerBinding::get(
11251124

11261125
viewTransitionDelegate->startViewTransition(
11271126
[&runtime, mutationFunc = std::move(mutationFunc)]() {
1127+
// mutationCallback is called synchronously by the
1128+
// delegate during startViewTransition, so &runtime is
1129+
// valid (it comes from the enclosing host function).
11281130
mutationFunc->call(runtime);
11291131
},
1130-
[readyResolveFunc = std::move(readyResolveFunc), &runtime]() {
1131-
if (*readyResolveFunc) {
1132-
(*readyResolveFunc)->call(runtime);
1133-
}
1132+
[readyResolveFunc = std::move(readyResolveFunc),
1133+
runtimeExecutor = uiManager->runtimeExecutor_]() mutable {
1134+
runtimeExecutor(
1135+
[readyResolveFunc = std::move(readyResolveFunc)](
1136+
jsi::Runtime& rt) mutable {
1137+
if (readyResolveFunc) {
1138+
readyResolveFunc->call(rt);
1139+
}
1140+
});
11341141
},
11351142
[finishedResolveFunc = std::move(finishedResolveFunc),
1136-
uiManager]() {
1137-
uiManager->runtimeExecutor_(
1143+
runtimeExecutor = uiManager->runtimeExecutor_]() {
1144+
runtimeExecutor(
11381145
[finishedResolveFunc = std::move(finishedResolveFunc)](
11391146
jsi::Runtime& rt) mutable {
1140-
if (*finishedResolveFunc) {
1141-
(*finishedResolveFunc)->call(rt);
1147+
if (finishedResolveFunc) {
1148+
finishedResolveFunc->call(rt);
11421149
}
11431150
});
11441151
});

packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerViewTransitionDelegate.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ class UIManagerViewTransitionDelegate {
2727

2828
virtual void restoreViewTransitionName(const ShadowNode &shadowNode) {}
2929

30+
/*
31+
* Start a view transition. mutationCallback MUST be called synchronously
32+
* on the JS thread before this method returns. onReadyCallback and
33+
* onCompleteCallback may be called from any thread.
34+
*/
3035
virtual void startViewTransition(
3136
std::function<void()> mutationCallback,
3237
std::function<void()> onReadyCallback,

0 commit comments

Comments
 (0)