Release long lived JSI objects ASAP (#12385)

vmoroz · jonthysell · web-flow · commit c7cc92b86285 · 2023-11-17T17:03:55.000-08:00
* Release long lived JSI objects ASAP

* Change files

* Fix BG thread scenario

* fix bad merge

---------

Co-authored-by: Jon Thysell &lt;jthysell@microsoft.com&gt;
diff --git a/change/react-native-windows-a2b5faff-fe2c-41f9-b341-3c02d3b4c12b.json b/change/react-native-windows-a2b5faff-fe2c-41f9-b341-3c02d3b4c12b.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Release long lived JSI objects ASAP",
+  "packageName": "react-native-windows",
+  "email": "vmorozov@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/vnext/Microsoft.ReactNative/JSDispatcherWriter.cpp b/vnext/Microsoft.ReactNative/JSDispatcherWriter.cpp
@@ -33,6 +33,12 @@ JSDispatcherWriter::JSDispatcherWriter(
     std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept
     : m_jsDispatcher(jsDispatcher), m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)) {}
 
+JSDispatcherWriter::~JSDispatcherWriter() {
+  if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
+    jsiRuntimeHolder->allowRelease();
+  }
+}
+
 void JSDispatcherWriter::WithResultArgs(
     Mso::Functor<void(facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t argCount)>
         handler) noexcept {
@@ -49,17 +55,18 @@ void JSDispatcherWriter::WithResultArgs(
     VerifyElseCrash(!m_jsiWriter);
     folly::dynamic dynValue = m_dynamicWriter->TakeValue();
     VerifyElseCrash(dynValue.isArray());
-    m_jsDispatcher.Post([handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder]() {
-      if (auto jsiRuntimeHolder = weakJsiRuntimeHolder.lock()) {
-        std::vector<facebook::jsi::Value> args;
-        args.reserve(dynValue.size());
-        auto &runtime = jsiRuntimeHolder->Runtime();
-        for (auto const &item : dynValue) {
-          args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
-        }
-        handler(runtime, args.data(), args.size());
-      }
-    });
+    m_jsDispatcher.Post(
+        [handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()]() {
+          if (auto jsiRuntimeHolder = weakJsiRuntimeHolder.lock()) {
+            std::vector<facebook::jsi::Value> args;
+            args.reserve(dynValue.size());
+            auto &runtime = jsiRuntimeHolder->Runtime();
+            for (auto const &item : dynValue) {
+              args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
+            }
+            handler(runtime, args.data(), args.size());
+          }
+        });
   }
 }
 
diff --git a/vnext/Microsoft.ReactNative/JSDispatcherWriter.h b/vnext/Microsoft.ReactNative/JSDispatcherWriter.h
@@ -14,6 +14,7 @@ namespace winrt::Microsoft::ReactNative {
 // In case if writing is done outside of JSDispatcher, it uses DynamicWriter to create
 // folly::dynamic which then is written to JsiWriter in JSDispatcher.
 struct JSDispatcherWriter : winrt::implements<JSDispatcherWriter, IJSValueWriter> {
+  ~JSDispatcherWriter();
   JSDispatcherWriter(
       IReactDispatcher const &jsDispatcher,
       std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept;
diff --git a/vnext/Microsoft.ReactNative/TurboModulesProvider.cpp b/vnext/Microsoft.ReactNative/TurboModulesProvider.cpp
@@ -221,11 +221,46 @@ class TurboModuleImpl : public facebook::react::TurboModule {
                   VerifyElseCrash(argCount > 1);
                   if (auto strongLongLivedObjectCollection = longLivedObjectCollection.lock()) {
                     auto jsiRuntimeHolder = LongLivedJsiRuntime::CreateWeak(strongLongLivedObjectCollection, rt);
+                    auto weakCallback1 = LongLivedJsiFunction::CreateWeak(
+                        strongLongLivedObjectCollection, rt, args[argCount - 2].getObject(rt).getFunction(rt));
+                    auto weakCallback2 = LongLivedJsiFunction::CreateWeak(
+                        strongLongLivedObjectCollection, rt, args[argCount - 1].getObject(rt).getFunction(rt));
+
                     method(
                         winrt::make<JsiReader>(rt, args, argCount - 2),
                         winrt::make<JSDispatcherWriter>(jsDispatcher, jsiRuntimeHolder),
-                        MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 2]),
-                        MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 1]));
+                        [weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
+                          writer.as<JSDispatcherWriter>()->WithResultArgs(
+                              [weakCallback1, weakCallback2, jsiRuntimeHolder](
+                                  facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
+                                if (auto callback1 = weakCallback1.lock()) {
+                                  callback1->Value().call(rt, args, count);
+                                  callback1->allowRelease();
+                                }
+                                if (auto callback2 = weakCallback2.lock()) {
+                                  callback2->allowRelease();
+                                }
+                                if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
+                                  runtimeHolder->allowRelease();
+                                }
+                              });
+                        },
+                        [weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
+                          writer.as<JSDispatcherWriter>()->WithResultArgs(
+                              [weakCallback1, weakCallback2, jsiRuntimeHolder](
+                                  facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
+                                if (auto callback2 = weakCallback2.lock()) {
+                                  callback2->Value().call(rt, args, count);
+                                  callback2->allowRelease();
+                                }
+                                if (auto callback1 = weakCallback1.lock()) {
+                                  callback1->allowRelease();
+                                }
+                                if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
+                                  runtimeHolder->allowRelease();
+                                }
+                              });
+                        });
                   }
                   return facebook::jsi::Value::undefined();
                 });
@@ -247,49 +282,65 @@ class TurboModuleImpl : public facebook::react::TurboModule {
                     auto argWriter = winrt::make<JSDispatcherWriter>(jsDispatcher, jsiRuntimeHolder);
                     return facebook::react::createPromiseAsJSIValue(
                         rt,
-                        [method, argReader, argWriter, strongLongLivedObjectCollection](
+                        [method, argReader, argWriter, strongLongLivedObjectCollection, jsiRuntimeHolder](
                             facebook::jsi::Runtime &runtime, std::shared_ptr<facebook::react::Promise> promise) {
+                          auto weakResolve = LongLivedJsiFunction::CreateWeak(
+                              strongLongLivedObjectCollection, runtime, std::move(promise->resolve_));
+                          auto weakReject = LongLivedJsiFunction::CreateWeak(
+                              strongLongLivedObjectCollection, runtime, std::move(promise->reject_));
                           method(
                               argReader,
                               argWriter,
-                              [weakResolve = LongLivedJsiFunction::CreateWeak(
-                                   strongLongLivedObjectCollection, runtime, std::move(promise->resolve_))](
-                                  const IJSValueWriter &writer) {
-                                writer.as<JSDispatcherWriter>()->WithResultArgs([weakResolve](
-                                                                                    facebook::jsi::Runtime &runtime,
-                                                                                    facebook::jsi::Value const *args,
-                                                                                    size_t argCount) {
-                                  VerifyElseCrash(argCount == 1);
-                                  if (auto resolveHolder = weakResolve.lock()) {
-                                    resolveHolder->Value().call(runtime, args[0]);
-                                  }
-                                });
+                              [weakResolve, weakReject, jsiRuntimeHolder](const IJSValueWriter &writer) {
+                                writer.as<JSDispatcherWriter>()->WithResultArgs(
+                                    [weakResolve, weakReject, jsiRuntimeHolder](
+                                        facebook::jsi::Runtime &runtime,
+                                        facebook::jsi::Value const *args,
+                                        size_t argCount) {
+                                      VerifyElseCrash(argCount == 1);
+                                      if (auto resolveHolder = weakResolve.lock()) {
+                                        resolveHolder->Value().call(runtime, args[0]);
+                                        resolveHolder->allowRelease();
+                                      }
+                                      if (auto rejectHolder = weakReject.lock()) {
+                                        rejectHolder->allowRelease();
+                                      }
+                                      if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
+                                        runtimeHolder->allowRelease();
+                                      }
+                                    });
                               },
-                              [weakReject = LongLivedJsiFunction::CreateWeak(
-                                   strongLongLivedObjectCollection, runtime, std::move(promise->reject_))](
-                                  const IJSValueWriter &writer) {
-                                writer.as<JSDispatcherWriter>()->WithResultArgs([weakReject](
-                                                                                    facebook::jsi::Runtime &runtime,
-                                                                                    facebook::jsi::Value const *args,
-                                                                                    size_t argCount) {
-                                  VerifyElseCrash(argCount == 1);
-                                  if (auto rejectHolder = weakReject.lock()) {
-                                    // To match the Android and iOS TurboModule behavior we create the Error object for
-                                    // the Promise rejection the same way as in updateErrorWithErrorData method.
-                                    // See react-native/Libraries/BatchedBridge/NativeModules.js for details.
-                                    auto error = runtime.global()
-                                                     .getPropertyAsFunction(runtime, "Error")
-                                                     .callAsConstructor(runtime, {});
-                                    auto &errorData = args[0];
-                                    if (errorData.isObject()) {
-                                      runtime.global()
-                                          .getPropertyAsObject(runtime, "Object")
-                                          .getPropertyAsFunction(runtime, "assign")
-                                          .call(runtime, error, errorData.getObject(runtime));
-                                    }
-                                    rejectHolder->Value().call(runtime, args[0]);
-                                  }
-                                });
+                              [weakResolve, weakReject, jsiRuntimeHolder](const IJSValueWriter &writer) {
+                                writer.as<JSDispatcherWriter>()->WithResultArgs(
+                                    [weakResolve, weakReject, jsiRuntimeHolder](
+                                        facebook::jsi::Runtime &runtime,
+                                        facebook::jsi::Value const *args,
+                                        size_t argCount) {
+                                      VerifyElseCrash(argCount == 1);
+                                      if (auto rejectHolder = weakReject.lock()) {
+                                        // To match the Android and iOS TurboModule behavior we create the Error object
+                                        // for the Promise rejection the same way as in updateErrorWithErrorData method.
+                                        // See react-native/Libraries/BatchedBridge/NativeModules.js for details.
+                                        auto error = runtime.global()
+                                                         .getPropertyAsFunction(runtime, "Error")
+                                                         .callAsConstructor(runtime, {});
+                                        auto &errorData = args[0];
+                                        if (errorData.isObject()) {
+                                          runtime.global()
+                                              .getPropertyAsObject(runtime, "Object")
+                                              .getPropertyAsFunction(runtime, "assign")
+                                              .call(runtime, error, errorData.getObject(runtime));
+                                        }
+                                        rejectHolder->Value().call(runtime, args[0]);
+                                        rejectHolder->allowRelease();
+                                      }
+                                      if (auto resolveHolder = weakResolve.lock()) {
+                                        resolveHolder->allowRelease();
+                                      }
+                                      if (auto runtimeHolder = jsiRuntimeHolder.lock()) {
+                                        runtimeHolder->allowRelease();
+                                      }
+                                    });
                               });
                         });
                   }
@@ -347,6 +398,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
           [weakCallback](facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t count) {
             if (auto callback = weakCallback.lock()) {
               callback->Value().call(rt, args, count);
+              callback->allowRelease();
             }
           });
     };
