From 19012ca9eeab456041ea0f9f8b5af0f5085ebb72 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 13 Feb 2024 08:14:17 -0800 Subject: [PATCH] RuntimeTarget refactor - ensure safe destruction of Instance Agents and Targets Summary: Changelog: [Internal] In D53233914 we copied PageTarget's approach for keeping track of its sessions into InstanceTarget (for keeping track of InstanceAgents). Here we complete that pattern by asserting that the agents are destroyed before their respective targets. NOTE: We might want to encapsulate this pattern in a helper/template class at some point. For now I'm going with the explicit approach. Reviewed By: hoxyq Differential Revision: D53266708 fbshipit-source-id: 4a90fde6c68e87d4667c44f81f8578a7a9072474 --- .../jsinspector-modern/InstanceTarget.cpp | 15 +++++++++++++++ .../jsinspector-modern/InstanceTarget.h | 1 + 2 files changed, 16 insertions(+) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp index 90f19df8dde..920973fa027 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp @@ -30,6 +30,21 @@ std::shared_ptr InstanceTarget::createAgent( return instanceAgent; } +void InstanceTarget::removeExpiredAgents() { + // Remove all expired agents. + forEachAgent([](auto&) {}); +} + +InstanceTarget::~InstanceTarget() { + removeExpiredAgents(); + + // Agents are owned by the session, not by InstanceTarget, but + // they hold an InstanceTarget& that we must guarantee is valid. + assert( + agents_.empty() && + "InstanceAgent objects must be destroyed before their InstanceTarget. Did you call PageTarget::unregisterInstance()?"); +} + RuntimeTarget& InstanceTarget::registerRuntime( RuntimeTargetDelegate& delegate) { assert(!currentRuntime_ && "Only one Runtime allowed"); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h index ae2ed58567a..53a215415f3 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h @@ -53,6 +53,7 @@ class InstanceTarget { InstanceTarget(InstanceTarget&&) = delete; InstanceTarget& operator=(const InstanceTarget&) = delete; InstanceTarget& operator=(InstanceTarget&&) = delete; + ~InstanceTarget(); std::shared_ptr createAgent( FrontendChannel channel,