From 1df38129337168d2968f4172a10b23bb88ab4c95 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 5 Mar 2024 06:32:45 -0800 Subject: [PATCH] Document extended RuntimeTargetDelegate lifetime requirements Summary: Changelog: [Internal] NOTE: This is a documentation-only diff. Documents that the caller of `InstanceTarget::registerRuntime()` is required to keep the `RuntimeTargetDelegate&` valid longer than previously stated. Rather than merely outliving the `RuntimeTarget` object ( = past `unregisterRuntime()`), the delegate must also remain valid while any JS is being executed in the underlying runtime. Proof that this requirement is already met by the existing integrations: * In Bridgeless, `RuntimeTargetDelegate` is implemented by [`JSRuntime`](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h#L20). JS execution via the `RuntimeExecutor` happens [here](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp#L70), while a [strong reference to the `JSRuntime`](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp#L66) is in scope. * In Bridge, `RuntimeTargetDelegate` is implemented by [`JSExecutor`](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/cxxreact/JSExecutor.h#L58). JS execution via the `RuntimeExecutor` happens [here](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/cxxreact/Instance.cpp#L253), while a (necessarily valid) non-owning [pointer to the `JSExecutor`](https://github.com/facebook/react-native/blob/9f85a249fa9e18eae745f103e390b2a8542e8ee9/packages/react-native/ReactCommon/cxxreact/Instance.cpp#L247) is in scope. bypass-github-export-checks Reviewed By: huntie Differential Revision: D54493456 fbshipit-source-id: c8dc11b0696e20b5fe9f3e16bb7591be0b3b6157 --- .../jsinspector-modern/InstanceTarget.h | 13 +++++++++++++ .../jsinspector-modern/RuntimeTarget.h | 16 +++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h index 101433c8b27..e9ec51b12db 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h @@ -70,9 +70,22 @@ class InstanceTarget : public EnableExecutorFromThis { FrontendChannel channel, SessionState& sessionState); + /** + * Registers a JS runtime with this InstanceTarget. \returns a reference to + * the created RuntimeTarget, which is owned by the \c InstanceTarget. All the + * requirements of \c RuntimeTarget::create must be met. + */ RuntimeTarget& registerRuntime( RuntimeTargetDelegate& delegate, RuntimeExecutor executor); + + /** + * Unregisters a JS runtime from this InstanceTarget. This destroys the \c + * RuntimeTarget, and it is no longer valid to use. Note that the \c + * RuntimeTargetDelegate& initially provided to \c registerRuntime may + * continue to be used as long as JavaScript execution continues in the + * runtime. + */ void unregisterRuntime(RuntimeTarget& runtime); private: diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h index 0e1160a7109..9960c170f03 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h @@ -77,15 +77,15 @@ class JSINSPECTOR_EXPORT RuntimeTarget : public EnableExecutorFromThis { public: /** - * Constructs a new RuntimeTarget. The caller must call setExecutor - * immediately afterwards. * \param executionContextDescription A description of the execution context * represented by this runtime. This is used for disambiguating the * source/destination of CDP messages when there are multiple runtimes * (concurrently or over the life of a Host). * \param delegate The object that will receive events from this target. The - * caller is responsible for - * ensuring that the delegate outlives this object. + * caller is responsible for ensuring that the delegate outlives this object + * AND that it remains valid for as long as the JS runtime is executing any + * code, even if the \c RuntimeTarget itself is destroyed. The delegate SHOULD + * be the object that owns the underlying jsi::Runtime, if any. * \param jsExecutor A RuntimeExecutor that can be used to schedule work on * the JS runtime's thread. The executor's queue should be empty when * RuntimeTarget is constructed (i.e. anything scheduled during the @@ -127,9 +127,11 @@ class JSINSPECTOR_EXPORT RuntimeTarget * represented by this runtime. This is used for disambiguating the * source/destination of CDP messages when there are multiple runtimes * (concurrently or over the life of a Host). - * \param delegate The object that will receive events from this target. - * The caller is responsible for ensuring that the delegate outlives this - * object. + * \param delegate The object that will receive events from this target. The + * caller is responsible for ensuring that the delegate outlives this object + * AND that it remains valid for as long as the JS runtime is executing any + * code, even if the \c RuntimeTarget itself is destroyed. The delegate SHOULD + * be the object that owns the underlying jsi::Runtime, if any. * \param jsExecutor A RuntimeExecutor that can be used to schedule work on * the JS runtime's thread. The executor's queue should be empty when * RuntimeTarget is constructed (i.e. anything scheduled during the