Skip to content

Commit 4331dc0

Browse files
committed
inspector: disconnect handles before releasing interfaces
Disconnect the thread handle while NodeInspectorClient still owns the interface. This prevents cross-thread posts from calling shared_from_this() after the interface's last strong reference has been released. Add a regression test verifying handle expiration during owner teardown. Fixes: #67079 Signed-off-by: Fatih Güzel <fatihguzeldev@gmail.com>
1 parent 20daf9a commit 4331dc0

5 files changed

Lines changed: 65 additions & 0 deletions

File tree

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@
490490
'test/cctest/test_quic_tokens.cc',
491491
],
492492
'node_cctest_inspector_sources': [
493+
'test/cctest/inspector/test_main_thread_interface.cc',
493494
'test/cctest/inspector/test_network_requests_buffer.cc',
494495
'test/cctest/inspector/test_node_protocol.cc',
495496
'test/cctest/test_inspector_socket.cc',

‎src/inspector/main_thread_interface.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,10 @@ class ThreadSafeDelegate : public InspectorSessionDelegate {
200200
MainThreadInterface::MainThreadInterface(Agent* agent) : agent_(agent) {}
201201

202202
MainThreadInterface::~MainThreadInterface() {
203+
Disconnect();
204+
}
205+
206+
void MainThreadInterface::Disconnect() {
203207
if (handle_)
204208
handle_->Reset();
205209
}

‎src/inspector/main_thread_interface.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class MainThreadInterface :
8080
explicit MainThreadInterface(Agent* agent);
8181
~MainThreadInterface();
8282

83+
void Disconnect();
8384
void DispatchMessages();
8485
void Post(std::unique_ptr<Request> request);
8586
bool WaitForFrontendEvent();

‎src/inspector_agent.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,11 @@ class NodeInspectorClient : public V8InspectorClient {
531531
contextCreated(env->context(), info);
532532
}
533533

534+
~NodeInspectorClient() override {
535+
// Close the handle before the interface can lose its last strong reference.
536+
if (interface_) interface_->Disconnect();
537+
}
538+
534539
void runMessageLoopOnPause(int context_group_id) override {
535540
waiting_for_resume_ = true;
536541
runMessageLoop();
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#include "gtest/gtest.h"
2+
#include "inspector/main_thread_interface.h"
3+
#include "inspector/worker_inspector.h"
4+
#include "node_test_fixture.h"
5+
6+
#include <memory>
7+
8+
namespace node {
9+
namespace inspector {
10+
namespace {
11+
12+
class MainThreadInterfaceTest : public EnvironmentTestFixture {};
13+
14+
class RetainInterfaceRequest : public Request {
15+
public:
16+
explicit RetainInterfaceRequest(
17+
std::shared_ptr<MainThreadInterface>* retained_interface)
18+
: retained_interface_(retained_interface) {}
19+
20+
void Call(MainThreadInterface* thread) override {
21+
*retained_interface_ = thread->shared_from_this();
22+
}
23+
24+
private:
25+
std::shared_ptr<MainThreadInterface>* retained_interface_;
26+
};
27+
28+
TEST_F(MainThreadInterfaceTest, ClosesHandleBeforeReleasingInterface) {
29+
const v8::HandleScope handle_scope(isolate_);
30+
const Argv argv;
31+
std::shared_ptr<MainThreadInterface> retained_interface;
32+
std::shared_ptr<MainThreadHandle> handle;
33+
{
34+
Env env{handle_scope, argv};
35+
handle = (*env)->inspector_agent()->GetWorkerManager()->MainThread();
36+
ASSERT_FALSE(handle->Expired());
37+
ASSERT_TRUE(handle->Post(
38+
std::make_unique<RetainInterfaceRequest>(&retained_interface)));
39+
(*env)->RunAndClearInterrupts();
40+
ASSERT_NE(retained_interface, nullptr);
41+
42+
// Keep the interface alive after its inspector owner is destroyed, so
43+
// closing the handle cannot depend on the interface's destructor.
44+
}
45+
46+
ASSERT_TRUE(handle->Expired());
47+
EXPECT_FALSE(handle->Post(
48+
std::make_unique<RetainInterfaceRequest>(&retained_interface)));
49+
retained_interface.reset();
50+
}
51+
52+
} // namespace
53+
} // namespace inspector
54+
} // namespace node

0 commit comments

Comments
 (0)