inspector: disconnect handles before releasing interfaces

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',

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();
 }

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> request);
   bool WaitForFrontendEvent();

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();

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 <memory>
+
+namespace node {
+namespace inspector {
+namespace {
+
+class MainThreadInterfaceTest : public EnvironmentTestFixture {};
+
+class RetainInterfaceRequest : public Request {
+ public:
+  explicit RetainInterfaceRequest(
+      std::shared_ptr<MainThreadInterface>* retained_interface)
+      : retained_interface_(retained_interface) {}
+
+  void Call(MainThreadInterface* thread) override {
+    *retained_interface_ = thread->shared_from_this();
+  }
+
+ private:
+  std::shared_ptr<MainThreadInterface>* retained_interface_;
+};
+
+TEST_F(MainThreadInterfaceTest, ClosesHandleBeforeReleasingInterface) {
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+  std::shared_ptr<MainThreadInterface> retained_interface;
+  std::shared_ptr<MainThreadHandle> handle;
+  {
+    Env env{handle_scope, argv};
+    handle = (*env)->inspector_agent()->GetWorkerManager()->MainThread();
+    ASSERT_FALSE(handle->Expired());
+    ASSERT_TRUE(handle->Post(
+        std::make_unique<RetainInterfaceRequest>(&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<RetainInterfaceRequest>(&retained_interface)));
+  retained_interface.reset();
+}
+
+}  // namespace
+}  // namespace inspector
+}  // namespace node