diff --git a/node.gyp b/node.gyp index 714609b03e66..cf97b25d8b30 100644 --- a/node.gyp +++ b/node.gyp @@ -490,6 +490,7 @@ 'test/cctest/test_quic_tokens.cc', ], 'node_cctest_inspector_sources': [ + 'test/cctest/inspector/test_main_thread_interface.cc', 'test/cctest/inspector/test_network_requests_buffer.cc', 'test/cctest/inspector/test_node_protocol.cc', 'test/cctest/test_inspector_socket.cc', diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index 0fca7483e95c..d4822598e17b 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -200,6 +200,10 @@ class ThreadSafeDelegate : public InspectorSessionDelegate { MainThreadInterface::MainThreadInterface(Agent* agent) : agent_(agent) {} MainThreadInterface::~MainThreadInterface() { + Disconnect(); +} + +void MainThreadInterface::Disconnect() { if (handle_) handle_->Reset(); } diff --git a/src/inspector/main_thread_interface.h b/src/inspector/main_thread_interface.h index 3311f9df014a..99fee99596c6 100644 --- a/src/inspector/main_thread_interface.h +++ b/src/inspector/main_thread_interface.h @@ -80,6 +80,7 @@ class MainThreadInterface : explicit MainThreadInterface(Agent* agent); ~MainThreadInterface(); + void Disconnect(); void DispatchMessages(); void Post(std::unique_ptr request); bool WaitForFrontendEvent(); diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 22506e26b32c..ac002e51d72f 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -531,6 +531,11 @@ class NodeInspectorClient : public V8InspectorClient { contextCreated(env->context(), info); } + ~NodeInspectorClient() override { + // Close the handle before the interface can lose its last strong reference. + if (interface_) interface_->Disconnect(); + } + void runMessageLoopOnPause(int context_group_id) override { waiting_for_resume_ = true; runMessageLoop(); diff --git a/test/cctest/inspector/test_main_thread_interface.cc b/test/cctest/inspector/test_main_thread_interface.cc new file mode 100644 index 000000000000..edb8dd57f976 --- /dev/null +++ b/test/cctest/inspector/test_main_thread_interface.cc @@ -0,0 +1,54 @@ +#include "gtest/gtest.h" +#include "inspector/main_thread_interface.h" +#include "inspector/worker_inspector.h" +#include "node_test_fixture.h" + +#include + +namespace node { +namespace inspector { +namespace { + +class MainThreadInterfaceTest : public EnvironmentTestFixture {}; + +class RetainInterfaceRequest : public Request { + public: + explicit RetainInterfaceRequest( + std::shared_ptr* retained_interface) + : retained_interface_(retained_interface) {} + + void Call(MainThreadInterface* thread) override { + *retained_interface_ = thread->shared_from_this(); + } + + private: + std::shared_ptr* retained_interface_; +}; + +TEST_F(MainThreadInterfaceTest, ClosesHandleBeforeReleasingInterface) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + std::shared_ptr retained_interface; + std::shared_ptr handle; + { + Env env{handle_scope, argv}; + handle = (*env)->inspector_agent()->GetWorkerManager()->MainThread(); + ASSERT_FALSE(handle->Expired()); + ASSERT_TRUE(handle->Post( + std::make_unique(&retained_interface))); + (*env)->RunAndClearInterrupts(); + ASSERT_NE(retained_interface, nullptr); + + // Keep the interface alive after its inspector owner is destroyed, so + // closing the handle cannot depend on the interface's destructor. + } + + ASSERT_TRUE(handle->Expired()); + EXPECT_FALSE(handle->Post( + std::make_unique(&retained_interface))); + retained_interface.reset(); +} + +} // namespace +} // namespace inspector +} // namespace node