Skip to content

Commit

Permalink
RuntimeTarget refactor - Add this-scoped async executors to all Tar…
Browse files Browse the repository at this point in the history
…gets

Summary:
Changelog: [Internal]

# This diff

1. Provides all Targets with an `executorFromThis()` method, which can be used from within a Target to access a *`this`-scoped main thread executor* = a `std::function` that will execute a callback asynchronously iff the current Target isn't destroyed first.
2. Refactors how (all) Target objects are constructed and retained, from a plain constructor to `static shared_ptr create()`. This is because `executorFromThis()` relies internally on `enable_shared_from_this` plus two-phase construction to populate the executor.
3. Creates utilities for deriving scoped executors from other executors and `shared_ptr`s.

The concept is very much like `RuntimeExecutor` in reverse: the facebook#1 use case is moving from the JS thread back to the main thread - where "main thread" is defined loosely as "anywhere it's legal to call methods on Target/Agent objects, access session state, etc". The actual dispatching mechanism is left up to the owner of each `PageTarget` object; for now we only have an iOS integration, where we use `RCTExecuteOnMainQueue`.

Coupling the ownership/lifetime semantics with task scheduling is helpful, because it avoids the footgun of accidentally/nondeterministically moving `shared_ptr`s (and destructors!) to a different thread/queue .

# This stack
I'm refactoring the way the Runtime concept works in the modern CDP backend to bring it in line with the Page/Instance concepts. 

Overall, this will let us:

* Integrate with engines that require us to instantiate a shared Target-like object (e.g. Hermes AsyncDebuggingAPI) in addition to an per-session Agent-like object.
* Access JSI in a CDP context (both at target setup/teardown time and during a CDP session) to implement our own engine-agnostic functionality (`console` interception, `Runtime.addBinding`, etc).
* Manage CDP execution contexts natively in RN, and (down the line) enable first-class debugging support for multiple Runtimes in an Instance.

The core diffs in this stack:

* ~~Introduce a `RuntimeTarget` class similar to `{Page,Instance}Target`. ~~
* ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). ~~
* ~~Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`.~~
* ~~Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`.~~
* ~~Provide `RuntimeTarget` and `RuntimeAgent` with primitives for safe JSI access, namely a `RuntimeExecutor` for scheduling work on the JS thread.~~ 
  * Provide RuntimeTarget with mechanism for scheduling work on the "main" thread from the JS thread, for when we need to do more than just send a CDP message (which we can already do with the thread-safe `FrontendChannel`) in response to a JS event. *← This diff*

## Architecture diagrams

Before this stack:
https://pxl.cl/4h7m0

After this stack:
https://pxl.cl/4h7m7

Reviewed By: hoxyq

Differential Revision: D53356953
  • Loading branch information
motiz88 authored and facebook-github-bot committed Feb 14, 2024
1 parent 31c25a4 commit 98fcbb6
Show file tree
Hide file tree
Showing 11 changed files with 253 additions and 54 deletions.
9 changes: 7 additions & 2 deletions packages/react-native/React/Base/RCTBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ @implementation RCTBridge {
NSURL *_delegateBundleURL;

std::unique_ptr<RCTBridgePageTargetDelegate> _inspectorPageDelegate;
std::unique_ptr<facebook::react::jsinspector_modern::PageTarget> _inspectorTarget;
std::shared_ptr<facebook::react::jsinspector_modern::PageTarget> _inspectorTarget;
std::optional<int> _inspectorPageId;
}

Expand Down Expand Up @@ -412,7 +412,12 @@ - (void)setUp

auto &inspectorFlags = facebook::react::jsinspector_modern::InspectorFlags::getInstance();
if (inspectorFlags.getEnableModernCDPRegistry() && !_inspectorPageId.has_value()) {
_inspectorTarget = std::make_unique<facebook::react::jsinspector_modern::PageTarget>(*_inspectorPageDelegate);
_inspectorTarget =
facebook::react::jsinspector_modern::PageTarget::create(*_inspectorPageDelegate, [](auto callback) {
RCTExecuteOnMainQueue(^{
callback();
});
});
__weak RCTBridge *weakSelf = self;
_inspectorPageId = facebook::react::jsinspector_modern::getInspectorInstance().addPage(
"React Native Bridge (Experimental)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@

namespace facebook::react::jsinspector_modern {

std::shared_ptr<InstanceTarget> InstanceTarget::create(
InstanceTargetDelegate& delegate,
VoidExecutor executor) {
std::shared_ptr<InstanceTarget> instanceTarget{new InstanceTarget(delegate)};
instanceTarget->setExecutor(executor);
return instanceTarget;
}

InstanceTarget::InstanceTarget(InstanceTargetDelegate& delegate)
: delegate_(delegate) {
(void)delegate_;
Expand All @@ -24,8 +32,7 @@ std::shared_ptr<InstanceAgent> InstanceTarget::createAgent(
SessionState& sessionState) {
auto instanceAgent =
std::make_shared<InstanceAgent>(channel, *this, sessionState);
instanceAgent->setCurrentRuntime(
currentRuntime_.has_value() ? &*currentRuntime_ : nullptr);
instanceAgent->setCurrentRuntime(currentRuntime_.get());
agents_.push_back(instanceAgent);
return instanceAgent;
}
Expand All @@ -47,9 +54,11 @@ InstanceTarget::~InstanceTarget() {

RuntimeTarget& InstanceTarget::registerRuntime(
RuntimeTargetDelegate& delegate,
RuntimeExecutor executor) {
RuntimeExecutor jsExecutor) {
assert(!currentRuntime_ && "Only one Runtime allowed");
currentRuntime_.emplace(delegate, executor);
currentRuntime_ = RuntimeTarget::create(
delegate, jsExecutor, makeVoidExecutor(executorFromThis()));

forEachAgent([currentRuntime = &*currentRuntime_](InstanceAgent& agent) {
agent.setCurrentRuntime(currentRuntime);
});
Expand All @@ -58,7 +67,7 @@ RuntimeTarget& InstanceTarget::registerRuntime(

void InstanceTarget::unregisterRuntime(RuntimeTarget& Runtime) {
assert(
currentRuntime_.has_value() && &currentRuntime_.value() == &Runtime &&
currentRuntime_ && currentRuntime_.get() == &Runtime &&
"Invalid unregistration");
forEachAgent([](InstanceAgent& agent) { agent.setCurrentRuntime(nullptr); });
currentRuntime_.reset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#pragma once

#include "RuntimeTarget.h"
#include "ScopedExecutor.h"
#include "SessionState.h"

#include <jsinspector-modern/InspectorInterfaces.h>
Expand Down Expand Up @@ -40,14 +41,20 @@ class InstanceTargetDelegate {
/**
* A Target that represents a single instance of React Native.
*/
class InstanceTarget final {
class InstanceTarget : public EnableExecutorFromThis<InstanceTarget> {
public:
/**
* Constructs a new InstanceTarget.
* \param delegate The object that will receive events from this target.
* The caller is responsible for ensuring that the delegate outlives this
* object.
* \param executor An executor that may be used to call methods on this
* InstanceTarget while it exists. \c create additionally guarantees that the
* executor will not be called after the InstanceTarget is destroyed.
*/
explicit InstanceTarget(InstanceTargetDelegate& delegate);
static std::shared_ptr<InstanceTarget> create(
InstanceTargetDelegate& delegate,
VoidExecutor executor);

InstanceTarget(const InstanceTarget&) = delete;
InstanceTarget(InstanceTarget&&) = delete;
Expand All @@ -65,8 +72,17 @@ class InstanceTarget final {
void unregisterRuntime(RuntimeTarget& runtime);

private:
/**
* Constructs a new InstanceTarget. The caller must call setExecutor
* immediately afterwards.
* \param delegate The object that will receive events from this target.
* The caller is responsible for ensuring that the delegate outlives this
* object.
*/
InstanceTarget(InstanceTargetDelegate& delegate);

InstanceTargetDelegate& delegate_;
std::optional<RuntimeTarget> currentRuntime_{std::nullopt};
std::shared_ptr<RuntimeTarget> currentRuntime_{nullptr};
std::list<std::weak_ptr<InstanceAgent>> agents_;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,22 @@ class PageTargetSession {
SessionState state_;
};

std::shared_ptr<PageTarget> PageTarget::create(
PageTargetDelegate& delegate,
VoidExecutor executor) {
std::shared_ptr<PageTarget> pageTarget{new PageTarget(delegate)};
pageTarget->setExecutor(executor);
return pageTarget;
}

PageTarget::PageTarget(PageTargetDelegate& delegate) : delegate_(delegate) {}

std::unique_ptr<ILocalConnection> PageTarget::connect(
std::unique_ptr<IRemoteConnection> connectionToFrontend,
SessionMetadata sessionMetadata) {
auto session = std::make_shared<PageTargetSession>(
std::move(connectionToFrontend), controller_, std::move(sessionMetadata));
session->setCurrentInstance(currentInstance_ ? &*currentInstance_ : nullptr);
session->setCurrentInstance(currentInstance_.get());
sessions_.push_back(std::weak_ptr(session));
return std::make_unique<CallbackLocalConnection>(
[session](std::string message) { (*session)(message); });
Expand All @@ -131,7 +139,8 @@ PageTargetDelegate::~PageTargetDelegate() {}

InstanceTarget& PageTarget::registerInstance(InstanceTargetDelegate& delegate) {
assert(!currentInstance_ && "Only one instance allowed");
currentInstance_.emplace(delegate);
currentInstance_ =
InstanceTarget::create(delegate, makeVoidExecutor(executorFromThis()));
forEachSession(
[currentInstance = &*currentInstance_](PageTargetSession& session) {
session.setCurrentInstance(currentInstance);
Expand All @@ -141,7 +150,7 @@ InstanceTarget& PageTarget::registerInstance(InstanceTargetDelegate& delegate) {

void PageTarget::unregisterInstance(InstanceTarget& instance) {
assert(
currentInstance_.has_value() && &currentInstance_.value() == &instance &&
currentInstance_ && currentInstance_.get() == &instance &&
"Invalid unregistration");
forEachSession(
[](PageTargetSession& session) { session.setCurrentInstance(nullptr); });
Expand Down
32 changes: 24 additions & 8 deletions packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#pragma once

#include "ScopedExecutor.h"

#include <jsinspector-modern/InspectorInterfaces.h>
#include <jsinspector-modern/InstanceTarget.h>

Expand Down Expand Up @@ -95,19 +97,25 @@ class PageTargetController final {
* "Host" in React Native's architecture - the entity that manages the
* lifecycle of a React Instance.
*/
class JSINSPECTOR_EXPORT PageTarget final {
class JSINSPECTOR_EXPORT PageTarget
: public EnableExecutorFromThis<PageTarget> {
public:
struct SessionMetadata {
std::optional<std::string> integrationName;
};

/**
* Constructs a new PageTarget.
* \param delegate The PageTargetDelegate that will receive events from
* this PageTarget. The caller is responsible for ensuring that the
* PageTargetDelegate outlives this object.
* \param delegate The PageTargetDelegate that will
* receive events from this PageTarget. The caller is responsible for ensuring
* that the PageTargetDelegate outlives this object.
* \param executor An executor that may be used to call methods on this
* PageTarget while it exists. \c create additionally guarantees that the
* executor will not be called after the PageTarget is destroyed.
*/
explicit PageTarget(PageTargetDelegate& delegate);
static std::shared_ptr<PageTarget> create(
PageTargetDelegate& delegate,
VoidExecutor executor);

PageTarget(const PageTarget&) = delete;
PageTarget(PageTarget&&) = delete;
Expand Down Expand Up @@ -146,11 +154,19 @@ class JSINSPECTOR_EXPORT PageTarget final {
void unregisterInstance(InstanceTarget& instance);

private:
std::list<std::weak_ptr<PageTargetSession>> sessions_;
/**
* Constructs a new PageTarget.
* The caller must call setExecutor immediately afterwards.
* \param delegate The PageTargetDelegate that will
* receive events from this PageTarget. The caller is responsible for ensuring
* that the PageTargetDelegate outlives this object.
*/
PageTarget(PageTargetDelegate& delegate);

PageTargetDelegate& delegate_;
std::list<std::weak_ptr<PageTargetSession>> sessions_;
PageTargetController controller_{*this};
std::optional<InstanceTarget> currentInstance_{std::nullopt};
std::shared_ptr<InstanceTarget> currentInstance_{nullptr};

/**
* Call the given function for every active session, and clean up any
Expand All @@ -175,7 +191,7 @@ class JSINSPECTOR_EXPORT PageTarget final {
}

inline bool hasInstance() const {
return currentInstance_.has_value();
return currentInstance_ != nullptr;
}

// Necessary to allow PageAgent to access PageTarget's internals in a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@
#include <jsinspector-modern/InstanceTarget.h>
#include <jsinspector-modern/PageTarget.h>
#include <jsinspector-modern/RuntimeTarget.h>
#include <jsinspector-modern/ScopedExecutor.h>
#include <jsinspector-modern/SessionState.h>
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,20 @@

namespace facebook::react::jsinspector_modern {

std::shared_ptr<RuntimeTarget> RuntimeTarget::create(
RuntimeTargetDelegate& delegate,
RuntimeExecutor jsExecutor,
VoidExecutor selfExecutor) {
std::shared_ptr<RuntimeTarget> runtimeTarget{
new RuntimeTarget(delegate, jsExecutor)};
runtimeTarget->setExecutor(selfExecutor);
return runtimeTarget;
}

RuntimeTarget::RuntimeTarget(
RuntimeTargetDelegate& delegate,
RuntimeExecutor executor)
: delegate_(delegate), executor_(executor) {}
RuntimeExecutor jsExecutor)
: delegate_(delegate), jsExecutor_(jsExecutor) {}

std::shared_ptr<RuntimeAgent> RuntimeTarget::createAgent(
FrontendChannel channel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <ReactCommon/RuntimeExecutor.h>
#include "InspectorInterfaces.h"
#include "RuntimeAgent.h"
#include "ScopedExecutor.h"
#include "SessionState.h"

#include <list>
Expand Down Expand Up @@ -48,18 +49,27 @@ class RuntimeTargetDelegate {
/**
* A Target corresponding to a JavaScript runtime.
*/
class JSINSPECTOR_EXPORT RuntimeTarget final {
class JSINSPECTOR_EXPORT RuntimeTarget
: public EnableExecutorFromThis<RuntimeTarget> {
public:
/**
* Constructs a new RuntimeTarget. The caller must call setExecutor
* immediately afterwards.
* \param delegate The object that will receive events from this target.
* The caller is responsible for ensuring that the delegate outlives this
* object.
* \param executor A RuntimeExecutor that can be used to schedule work on
* \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
* constructor should be executed before any user code is run).
* \param selfExecutor An executor that may be used to call methods on this
* RuntimeTarget while it exists. \c create additionally guarantees that the
* executor will not be called after the RuntimeTarget is destroyed.
*/
RuntimeTarget(RuntimeTargetDelegate& delegate, RuntimeExecutor executor);
static std::shared_ptr<RuntimeTarget> create(
RuntimeTargetDelegate& delegate,
RuntimeExecutor jsExecutor,
VoidExecutor selfExecutor);

RuntimeTarget(const RuntimeTarget&) = delete;
RuntimeTarget(RuntimeTarget&&) = delete;
Expand All @@ -81,8 +91,21 @@ class JSINSPECTOR_EXPORT RuntimeTarget final {
SessionState& sessionState);

private:
/**
* Constructs a new RuntimeTarget. The caller must call setExecutor
* immediately afterwards.
* \param delegate The object that will receive events from this target.
* The caller is responsible for ensuring that the delegate outlives this
* object.
* \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
* constructor should be executed before any user code is run).
*/
RuntimeTarget(RuntimeTargetDelegate& delegate, RuntimeExecutor jsExecutor);

RuntimeTargetDelegate& delegate_;
RuntimeExecutor executor_;
RuntimeExecutor jsExecutor_;
std::list<std::weak_ptr<RuntimeAgent>> agents_;

/**
Expand Down
Loading

0 comments on commit 98fcbb6

Please sign in to comment.