diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp index 5cf43e44842..ce52946f8d0 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +#include #include "JsiIntegrationTest.h" #include "engines/JsiIntegrationTestHermesEngineAdapter.h" @@ -44,12 +45,13 @@ struct Params { /** * A test fixture for the Console API. */ -class ConsoleApiTest - : public JsiIntegrationPortableTest, - public WithParamInterface { +class ConsoleApiTest : public JsiIntegrationPortableTestBase< + JsiIntegrationTestHermesEngineAdapter, + folly::QueuedImmediateExecutor>, + public WithParamInterface { protected: void SetUp() override { - JsiIntegrationPortableTest::SetUp(); + JsiIntegrationPortableTestBase::SetUp(); connect(); EXPECT_CALL( fromPage(), @@ -81,7 +83,7 @@ class ConsoleApiTest if (!GetParam().runtimeEnabledAtStart) { enableRuntimeDomain(); } - JsiIntegrationPortableTest::TearDown(); + JsiIntegrationPortableTestBase::TearDown(); } /** diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp index c83486375da..851cea03f20 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp @@ -6,6 +6,8 @@ */ #include +#include +#include #include "JsiIntegrationTest.h" #include "engines/JsiIntegrationTestGenericEngineAdapter.h" @@ -31,11 +33,40 @@ using AllEngines = Types< using AllHermesVariants = Types; +template +using JsiIntegrationPortableTest = JsiIntegrationPortableTestBase< + EngineAdapter, + folly::QueuedImmediateExecutor>; + TYPED_TEST_SUITE(JsiIntegrationPortableTest, AllEngines); template -using JsiIntegrationHermesTest = JsiIntegrationPortableTest; +using JsiIntegrationHermesTest = JsiIntegrationPortableTestBase< + EngineAdapter, + folly::QueuedImmediateExecutor>; + +/** + * Fixture class for tests that run on a ManualExecutor. Work scheduled + * on the executor is *not* run automatically; it must be manually advanced + * in the body of the test. + */ +template +class JsiIntegrationHermesTestAsync : public JsiIntegrationPortableTestBase< + EngineAdapter, + folly::ManualExecutor> { + public: + void TearDown() override { + // Assert there are no pending tasks on the ManualExecutor. + auto tasksCleared = this->executor_.clear(); + EXPECT_EQ(tasksCleared, 0) + << "There were still pending tasks on executor_ at the end of the test. Use advance() or run() as needed."; + JsiIntegrationPortableTestBase:: + TearDown(); + } +}; + TYPED_TEST_SUITE(JsiIntegrationHermesTest, AllHermesVariants); +TYPED_TEST_SUITE(JsiIntegrationHermesTestAsync, AllHermesVariants); #pragma region AllEngines @@ -371,6 +402,56 @@ TYPED_TEST(JsiIntegrationPortableTest, ReactNativeApplicationDisable) { #pragma endregion // AllEngines #pragma region AllHermesVariants +TYPED_TEST(JsiIntegrationHermesTestAsync, HermesObjectsTableDoesNotMemoryLeak) { + // This is a regression test for T186157855 (CDPAgent leaking JSI data in + // RemoteObjectsTable past the Runtime's lifetime) + this->connect(); + this->executor_.run(); + + InSequence s; + + this->expectMessageFromPage(JsonParsed( + AllOf(AtJsonPtr("/method", "Runtime.executionContextCreated")))); + this->expectMessageFromPage(JsonEq(R"({ + "id": 1, + "result": {} + })")); + this->toPage_->sendMessage(R"({ + "id": 1, + "method": "Runtime.enable" + })"); + this->executor_.run(); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Runtime.consoleAPICalled"), + AtJsonPtr("/params/args/0/objectId", "1")))); + this->eval(R"(console.log({a: 1});)"); + this->executor_.run(); + + this->expectMessageFromPage(JsonEq(R"({ + "method": "Runtime.executionContextDestroyed", + "params": { + "executionContextId": 1 + } + })")); + this->expectMessageFromPage(JsonEq(R"({ + "method": "Runtime.executionContextsCleared" + })")); + this->expectMessageFromPage(JsonEq(R"({ + "method": "Runtime.executionContextCreated", + "params": { + "context": { + "id": 2, + "origin": "", + "name": "main" + } + } + })")); + // NOTE: Doesn't crash when Hermes checks for JSI value leaks + this->reload(); + this->executor_.run(); +} + TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpression) { this->connect(); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h index 10132b6425d..49306899d0d 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h @@ -8,7 +8,6 @@ #pragma once #include -#include #include #include #include @@ -42,15 +41,15 @@ namespace facebook::react::jsinspector_modern { * for a particular engine, plus exposes access to a RuntimeExecutor (based on * the provided folly::Executor) and the corresponding jsi::Runtime. */ -template -class JsiIntegrationPortableTest : public ::testing::Test, - private HostTargetDelegate { - folly::QueuedImmediateExecutor immediateExecutor_; - +template +class JsiIntegrationPortableTestBase : public ::testing::Test, + private HostTargetDelegate { protected: - JsiIntegrationPortableTest() + Executor executor_; + + JsiIntegrationPortableTestBase() : inspectorFlagsGuard_{EngineAdapter::getInspectorFlagOverrides()}, - engineAdapter_{immediateExecutor_} {} + engineAdapter_{executor_} {} void SetUp() override { // NOTE: Using SetUp() so we can call virtual methods like @@ -63,7 +62,7 @@ class JsiIntegrationPortableTest : public ::testing::Test, loadMainBundle(); } - ~JsiIntegrationPortableTest() override { + ~JsiIntegrationPortableTestBase() override { toPage_.reset(); if (runtimeTarget_) { EXPECT_TRUE(instance_); @@ -118,7 +117,7 @@ class JsiIntegrationPortableTest : public ::testing::Test, instance_ = nullptr; } // Recreate the engine (e.g. to wipe any state in the inner jsi::Runtime) - engineAdapter_.emplace(immediateExecutor_); + engineAdapter_.emplace(executor_); instance_ = &page_->registerInstance(instanceTargetDelegate_); setupRuntimeBeforeRegistration(engineAdapter_->getRuntime()); runtimeTarget_ = &instance_->registerRuntime( @@ -133,7 +132,7 @@ class JsiIntegrationPortableTest : public ::testing::Test, } VoidExecutor inspectorExecutor_ = [this](auto callback) { - immediateExecutor_.add(callback); + executor_.add(callback); }; jsi::Value eval(std::string_view code) {