From 5c7cbebfeee7c807368457148cfeae9cfe48104e Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 8 Oct 2025 17:38:42 -0700 Subject: [PATCH] Refactor console API implementation to use tryExecuteSync for clarity (#54069) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/54069 Changelog: [Internal] D83238216 added a `tryExecuteSync` function for use with `EnableExecutorFromThis` objects - concretely, for calling functions on a `weak_ptr` from the JS thread while ensuring the `RuntimeTarget` is always destroyed on the inspector thread. `tryExecuteSync` is a generalisation of the lambda-based `delegateExecutorSync` helper from `RuntimeTargetConsole`, so in this diff we refactor the latter to use the more general and better-documented function. Reviewed By: huntie Differential Revision: D83838062 fbshipit-source-id: 85fd5a43e204cc634b573e2a3bda47a9ec523fca --- .../RuntimeTargetConsole.cpp | 38 +++++-------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp index 6b30b988631..25a6e694c17 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp @@ -525,30 +525,6 @@ void RuntimeTarget::installConsoleHandler() { auto console = objectCreate(runtime, std::move(consolePrototype)); auto state = std::make_shared(); - /** - * An executor that runs synchronously and provides a safe reference to our - * RuntimeTargetDelegate for use on the JS thread. - * \see RuntimeTargetDelegate for information on which methods are safe to - * call on the JS thread. - * \warning The callback will not run if the RuntimeTarget has been - * destroyed. - */ - auto delegateExecutorSync = - [selfWeak, - selfExecutor](std::invocable auto func) { - if (auto self = selfWeak.lock()) { - // Q: Why is it safe to use self->delegate_ here? - // A: Because the caller of InspectorTarget::registerRuntime - // is explicitly required to guarantee that the delegate not - // only outlives the target, but also outlives all JS code - // execution that occurs on the JS thread. - func(self->delegate_); - // To ensure we never destroy `self` on the JS thread, send - // our shared_ptr back to the inspector thread. - selfExecutor([self = std::move(self)](auto&) { (void)self; }); - } - }; - /** * Install a console method with the given name and body. The body receives * the usual JSI host function parameters plus a ConsoleState reference, a @@ -569,20 +545,26 @@ void RuntimeTarget::installConsoleHandler() { forwardToOriginalConsole( originalConsole, methodName, - [body = std::move(body), state, delegateExecutorSync]( + [body = std::move(body), state, selfWeak]( jsi::Runtime& runtime, const jsi::Value& /*thisVal*/, const jsi::Value* args, size_t count) { auto timestampMs = getTimestampMs(); - delegateExecutorSync([&](auto& runtimeTargetDelegate) { - auto stackTrace = runtimeTargetDelegate.captureStackTrace( + tryExecuteSync(selfWeak, [&](auto& self) { + // Q: Why is it safe to use self->delegate_ here? + // A: Because the caller of + // InspectorTarget::registerRuntime is explicitly required + // to guarantee that the delegate not only outlives the + // target, but also outlives all JS code execution that + // occurs on the JS thread. + auto stackTrace = self.delegate_.captureStackTrace( runtime, /* framesToSkip */ 1); body( runtime, args, count, - runtimeTargetDelegate, + self.delegate_, *state, timestampMs, std::move(stackTrace));