Skip to content

Commit c7cc92b

Browse files
vmorozjonthysell
vmoroz
and
jonthysell
authored
Release long lived JSI objects ASAP (#12385)
* Release long lived JSI objects ASAP * Change files * Fix BG thread scenario * fix bad merge --------- Co-authored-by: Jon Thysell <[email protected]>
1 parent b84ca2d commit c7cc92b

File tree

4 files changed

+118
-51
lines changed

4 files changed

+118
-51
lines changed

‎change/react-native-windows-a2b5faff-fe2c-41f9-b341-3c02d3b4c12b.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Release long lived JSI objects ASAP",
4+
"packageName": "react-native-windows",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎vnext/Microsoft.ReactNative/JSDispatcherWriter.cpp

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ JSDispatcherWriter::JSDispatcherWriter(
3333
std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept
3434
: m_jsDispatcher(jsDispatcher), m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)) {}
3535

36+
JSDispatcherWriter::~JSDispatcherWriter() {
37+
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
38+
jsiRuntimeHolder->allowRelease();
39+
}
40+
}
41+
3642
void JSDispatcherWriter::WithResultArgs(
3743
Mso::Functor<void(facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t argCount)>
3844
handler) noexcept {
@@ -49,17 +55,18 @@ void JSDispatcherWriter::WithResultArgs(
4955
VerifyElseCrash(!m_jsiWriter);
5056
folly::dynamic dynValue = m_dynamicWriter->TakeValue();
5157
VerifyElseCrash(dynValue.isArray());
52-
m_jsDispatcher.Post([handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder]() {
53-
if (auto jsiRuntimeHolder = weakJsiRuntimeHolder.lock()) {
54-
std::vector<facebook::jsi::Value> args;
55-
args.reserve(dynValue.size());
56-
auto &runtime = jsiRuntimeHolder->Runtime();
57-
for (auto const &item : dynValue) {
58-
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
59-
}
60-
handler(runtime, args.data(), args.size());
61-
}
62-
});
58+
m_jsDispatcher.Post(
59+
[handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()]() {
60+
if (auto jsiRuntimeHolder = weakJsiRuntimeHolder.lock()) {
61+
std::vector<facebook::jsi::Value> args;
62+
args.reserve(dynValue.size());
63+
auto &runtime = jsiRuntimeHolder->Runtime();
64+
for (auto const &item : dynValue) {
65+
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
66+
}
67+
handler(runtime, args.data(), args.size());
68+
}
69+
});
6370
}
6471
}
6572

‎vnext/Microsoft.ReactNative/JSDispatcherWriter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace winrt::Microsoft::ReactNative {
1414
// In case if writing is done outside of JSDispatcher, it uses DynamicWriter to create
1515
// folly::dynamic which then is written to JsiWriter in JSDispatcher.
1616
struct JSDispatcherWriter : winrt::implements<JSDispatcherWriter, IJSValueWriter> {
17+
~JSDispatcherWriter();
1718
JSDispatcherWriter(
1819
IReactDispatcher const &jsDispatcher,
1920
std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept;

‎vnext/Microsoft.ReactNative/TurboModulesProvider.cpp

Lines changed: 92 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,46 @@ class TurboModuleImpl : public facebook::react::TurboModule {
221221
VerifyElseCrash(argCount > 1);
222222
if (auto strongLongLivedObjectCollection = longLivedObjectCollection.lock()) {
223223
auto jsiRuntimeHolder = LongLivedJsiRuntime::CreateWeak(strongLongLivedObjectCollection, rt);
224+
auto weakCallback1 = LongLivedJsiFunction::CreateWeak(
225+
strongLongLivedObjectCollection, rt, args[argCount - 2].getObject(rt).getFunction(rt));
226+
auto weakCallback2 = LongLivedJsiFunction::CreateWeak(
227+
strongLongLivedObjectCollection, rt, args[argCount - 1].getObject(rt).getFunction(rt));
228+
224229
method(
225230
winrt::make<JsiReader>(rt, args, argCount - 2),
226231
winrt::make<JSDispatcherWriter>(jsDispatcher, jsiRuntimeHolder),
227-
MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 2]),
228-
MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 1]));
232+
[weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
233+
writer.as<JSDispatcherWriter>()->WithResultArgs(
234+
[weakCallback1, weakCallback2, jsiRuntimeHolder](
235+
facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
236+
if (auto callback1 = weakCallback1.lock()) {
237+
callback1->Value().call(rt, args, count);
238+
callback1->allowRelease();
239+
}
240+
if (auto callback2 = weakCallback2.lock()) {
241+
callback2->allowRelease();
242+
}
243+
if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
244+
runtimeHolder->allowRelease();
245+
}
246+
});
247+
},
248+
[weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
249+
writer.as<JSDispatcherWriter>()->WithResultArgs(
250+
[weakCallback1, weakCallback2, jsiRuntimeHolder](
251+
facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
252+
if (auto callback2 = weakCallback2.lock()) {
253+
callback2->Value().call(rt, args, count);
254+
callback2->allowRelease();
255+
}
256+
if (auto callback1 = weakCallback1.lock()) {
257+
callback1->allowRelease();
258+
}
259+
if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
260+
runtimeHolder->allowRelease();
261+
}
262+
});
263+
});
229264
}
230265
return facebook::jsi::Value::undefined();
231266
});
@@ -247,49 +282,65 @@ class TurboModuleImpl : public facebook::react::TurboModule {
247282
auto argWriter = winrt::make<JSDispatcherWriter>(jsDispatcher, jsiRuntimeHolder);
248283
return facebook::react::createPromiseAsJSIValue(
249284
rt,
250-
[method, argReader, argWriter, strongLongLivedObjectCollection](
285+
[method, argReader, argWriter, strongLongLivedObjectCollection, jsiRuntimeHolder](
251286
facebook::jsi::Runtime &runtime, std::shared_ptr<facebook::react::Promise> promise) {
287+
auto weakResolve = LongLivedJsiFunction::CreateWeak(
288+
strongLongLivedObjectCollection, runtime, std::move(promise->resolve_));
289+
auto weakReject = LongLivedJsiFunction::CreateWeak(
290+
strongLongLivedObjectCollection, runtime, std::move(promise->reject_));
252291
method(
253292
argReader,
254293
argWriter,
255-
[weakResolve = LongLivedJsiFunction::CreateWeak(
256-
strongLongLivedObjectCollection, runtime, std::move(promise->resolve_))](
257-
const IJSValueWriter &writer) {
258-
writer.as<JSDispatcherWriter>()->WithResultArgs([weakResolve](
259-
facebook::jsi::Runtime &runtime,
260-
facebook::jsi::Value const *args,
261-
size_t argCount) {
262-
VerifyElseCrash(argCount == 1);
263-
if (auto resolveHolder = weakResolve.lock()) {
264-
resolveHolder->Value().call(runtime, args[0]);
265-
}
266-
});
294+
[weakResolve, weakReject, jsiRuntimeHolder](const IJSValueWriter &writer) {
295+
writer.as<JSDispatcherWriter>()->WithResultArgs(
296+
[weakResolve, weakReject, jsiRuntimeHolder](
297+
facebook::jsi::Runtime &runtime,
298+
facebook::jsi::Value const *args,
299+
size_t argCount) {
300+
VerifyElseCrash(argCount == 1);
301+
if (auto resolveHolder = weakResolve.lock()) {
302+
resolveHolder->Value().call(runtime, args[0]);
303+
resolveHolder->allowRelease();
304+
}
305+
if (auto rejectHolder = weakReject.lock()) {
306+
rejectHolder->allowRelease();
307+
}
308+
if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
309+
runtimeHolder->allowRelease();
310+
}
311+
});
267312
},
268-
[weakReject = LongLivedJsiFunction::CreateWeak(
269-
strongLongLivedObjectCollection, runtime, std::move(promise->reject_))](
270-
const IJSValueWriter &writer) {
271-
writer.as<JSDispatcherWriter>()->WithResultArgs([weakReject](
272-
facebook::jsi::Runtime &runtime,
273-
facebook::jsi::Value const *args,
274-
size_t argCount) {
275-
VerifyElseCrash(argCount == 1);
276-
if (auto rejectHolder = weakReject.lock()) {
277-
// To match the Android and iOS TurboModule behavior we create the Error object for
278-
// the Promise rejection the same way as in updateErrorWithErrorData method.
279-
// See react-native/Libraries/BatchedBridge/NativeModules.js for details.
280-
auto error = runtime.global()
281-
.getPropertyAsFunction(runtime, "Error")
282-
.callAsConstructor(runtime, {});
283-
auto &errorData = args[0];
284-
if (errorData.isObject()) {
285-
runtime.global()
286-
.getPropertyAsObject(runtime, "Object")
287-
.getPropertyAsFunction(runtime, "assign")
288-
.call(runtime, error, errorData.getObject(runtime));
289-
}
290-
rejectHolder->Value().call(runtime, args[0]);
291-
}
292-
});
313+
[weakResolve, weakReject, jsiRuntimeHolder](const IJSValueWriter &writer) {
314+
writer.as<JSDispatcherWriter>()->WithResultArgs(
315+
[weakResolve, weakReject, jsiRuntimeHolder](
316+
facebook::jsi::Runtime &runtime,
317+
facebook::jsi::Value const *args,
318+
size_t argCount) {
319+
VerifyElseCrash(argCount == 1);
320+
if (auto rejectHolder = weakReject.lock()) {
321+
// To match the Android and iOS TurboModule behavior we create the Error object
322+
// for the Promise rejection the same way as in updateErrorWithErrorData method.
323+
// See react-native/Libraries/BatchedBridge/NativeModules.js for details.
324+
auto error = runtime.global()
325+
.getPropertyAsFunction(runtime, "Error")
326+
.callAsConstructor(runtime, {});
327+
auto &errorData = args[0];
328+
if (errorData.isObject()) {
329+
runtime.global()
330+
.getPropertyAsObject(runtime, "Object")
331+
.getPropertyAsFunction(runtime, "assign")
332+
.call(runtime, error, errorData.getObject(runtime));
333+
}
334+
rejectHolder->Value().call(runtime, args[0]);
335+
rejectHolder->allowRelease();
336+
}
337+
if (auto resolveHolder = weakResolve.lock()) {
338+
resolveHolder->allowRelease();
339+
}
340+
if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
341+
runtimeHolder->allowRelease();
342+
}
343+
});
293344
});
294345
});
295346
}
@@ -347,6 +398,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
347398
[weakCallback](facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
348399
if (auto callback = weakCallback.lock()) {
349400
callback->Value().call(rt, args, count);
401+
callback->allowRelease();
350402
}
351403
});
352404
};

0 commit comments

Comments
 (0)