8000 inspector, trace_events: make sure messages are sent on a main thread · nodejs/node@6967407 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6967407

Browse files
Eugene Ostroukhovaddaleax
Eugene Ostroukhov
authored and
addaleax
committed
inspector, trace_events: make sure messages are sent on a main thread
Fixes: #23185 PR-URL: #24814 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4fe0819 commit 6967407

File tree

6 files changed

+117
-26
lines changed

6 files changed

+117
-26
lines changed

‎src/inspector/main_thread_interface.cc‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,15 +316,21 @@ void MainThreadInterface::RemoveObject(int id) {
316316
}
317317

318318
Deletable* MainThreadInterface::GetObject(int id) {
319-
auto iterator = managed_objects_.find(id);
319+
Deletable* pointer = GetObjectIfExists(id);
320320
// This would mean the object is requested after it was disposed, which is
321321
// a coding error.
322-
CHECK_NE(managed_objects_.end(), iterator);
323-
Deletable* pointer = iterator->second.get();
324322
CHECK_NE(nullptr, pointer);
325323
return pointer;
326324
}
327325

326+
Deletable* MainThreadInterface::GetObjectIfExists(int id) {
327+
auto iterator = managed_objects_.find(id);
328+
if (iterator == managed_objects_.end()) {
329+
return nullptr;
330+
}
331+
return iterator->second.get();
332+
}
333+
328334
std::unique_ptr<StringBuffer> Utf8ToStringView(const std::string& message) {
329335
icu::UnicodeString utf16 = icu::UnicodeString::fromUTF8(
330336
icu::StringPiece(message.data(), message.length()));

‎src/inspector/main_thread_interface.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class MainThreadInterface {
8484
}
8585
void AddObject(int handle, std::unique_ptr<Deletable> object);
8686
Deletable* GetObject(int id);
87+
Deletable* GetObjectIfExists(int id);
8788
void RemoveObject(int handle);
8889

8990
private:

‎src/inspector/tracing_agent.cc‎

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "tracing_agent.h"
2+
#include "main_thread_interface.h"
23
#include "node_internals.h"
34

45
#include "env-inl.h"
@@ -14,10 +15,76 @@ namespace protocol {
1415
namespace {
1516
using v8::platform::tracing::TraceWriter;
1617

18+
class DeletableFrontendWrapper : public Deletable {
19+
public:
20+
explicit DeletableFrontendWrapper(
21+
std::weak_ptr<NodeTracing::Frontend> frontend)
22+
: frontend_(frontend) {}
23+
24+
// This should only be called from the main thread, meaning frontend should
25+
// not be destroyed concurrently.
26+
NodeTracing::Frontend* get() { return frontend_.lock().get(); }
27+
28+
private:
29+
std::weak_ptr<NodeTracing::Frontend> frontend_;
30+
};
31+
32+
class CreateFrontendWrapperRequest : public Request {
33+
public:
34+
CreateFrontendWrapperRequest(int object_id,
35+
std::weak_ptr<NodeTracing::Frontend> frontend)
36+
: object_id_(object_id) {
37+
frontend_wrapper_ = std::make_unique<DeletableFrontendWrapper>(frontend);
38+
}
39+
40+
void Call(MainThreadInterface* thread) override {
41+
thread->AddObject(object_id_, std::move(frontend_wrapper_));
42+
}
43+
44+
private:
45+
int object_id_;
46+
std::unique_ptr<DeletableFrontendWrapper> frontend_wrapper_;
47+
};
48+
49+
class DestroyFrontendWrapperRequest : public Request {
50+
public:
51+
explicit DestroyFrontendWrapperRequest(int object_id)
52+
: object_id_(object_id) {}
53+
54+
void Call(MainThreadInterface* thread) override {
55+
thread->RemoveObject(object_id_);
56+
}
57+
58+
private:
59+
int object_id_;
60+
};
61+
62+
class SendMessageRequest : public Request {
63+
public:
64+
explicit SendMessageRequest(int object_id, const std::string& message)
65+
: object_id_(object_id), message_(message) {}
66+
67+
void Call(MainThreadInterface* thread) override {
68+
DeletableFrontendWrapper* frontend_wrapper =
69+
static_cast<DeletableFrontendWrapper*>(
70+
thread->GetObjectIfExists(object_id_));
71+
if (frontend_wrapper == nullptr) return;
72+
auto frontend = frontend_wrapper->get();
73+
if (frontend != nullptr) {
74+
frontend->sendRawNotification(message_);
75+
}
76+
}
77+
78+
private:
79+
int object_id_;
80+
std::string message_;
81+
};
82+
1783
class InspectorTraceWriter : public node::tracing::AsyncTraceWriter {
1884
public:
19-
explicit InspectorTraceWriter(NodeTracing::Frontend* frontend)
20-
: frontend_(frontend) {}
85+
explicit InspectorTraceWriter(int frontend_object_id,
86+
std::shared_ptr<MainThreadHandle> main_thread)
87+
: frontend_object_id_(frontend_object_id), main_thread_(main_thread) {}
2188

2289
void AppendTraceEvent(
2390
v8::platform::tracing::TraceObject* trace_event) override {
@@ -35,27 +102,35 @@ class InspectorTraceWriter : public node::tracing::AsyncTraceWriter {
35102
std::ostringstream::ate);
36103
result << stream_.str();
37104
result << "}";
38-
frontend_->sendRawNotification(result.str());
105+
main_thread_->Post(std::make_unique<SendMessageRequest>(frontend_object_id_,
106+
result.str()));
39107
stream_.str("");
40108
}
41109

42110
private:
43111
std::unique_ptr<TraceWriter> json_writer_;
44112
std::ostringstream stream_;
45-
NodeTracing::Frontend* frontend_;
113+
int frontend_object_id_;
114+
std::shared_ptr<MainThreadHandle> main_thread_;
46115
};
47116
} // namespace
48117

49-
TracingAgent::TracingAgent(Environment* env)
50-
: env_(env) {
51-
}
118+
TracingAgent::TracingAgent(Environment* env,
119+
std::shared_ptr<MainThreadHandle> main_thread)
120 A3E2 +
: env_(env), main_thread_(main_thread) {}
52121

53122
TracingAgent::~TracingAgent() {
54123
trace_writer_.reset();
124+
main_thread_->Post(
125+
std::make_unique<DestroyFrontendWrapperRequest>(frontend_object_id_));
55126
}
56127

57128
void TracingAgent::Wire(UberDispatcher* dispatcher) {
58-
frontend_.reset(new NodeTracing::Frontend(dispatcher->channel()));
129+
// Note that frontend is still owned by TracingAgent
130+
frontend_ = std::make_shared<NodeTracing::Frontend>(dispatcher->channel());
131+
frontend_object_id_ = main_thread_->newObjectId();
132+
main_thread_->Post(std::make_unique<CreateFrontendWrapperRequest>(
133+
frontend_object_id_, frontend_));
59134
NodeTracing::Dispatcher::wire(dispatcher, this);
60135
}
61136

@@ -81,11 +156,11 @@ DispatchResponse TracingAgent::start(
81156

82157
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
83158
if (writer != nullptr) {
84-
trace_writer_ = writer->agent()->AddClient(
85-
categories_set,
86-
std::unique_ptr<InspectorTraceWriter>(
87-
new InspectorTraceWriter(frontend_.get())),
88-
tracing::Agent::kIgnoreDefaultCategories);
159+
trace_writer_ =
160+
writer->agent()->AddClient(categories_set,
161+
std::make_unique<InspectorTraceWriter>(
162+
frontend_object_id_, main_thread_),
163+
tracing::Agent::kIgnoreDefaultCategories);
89164
}
90165
return DispatchResponse::OK();
91166
}

‎src/inspector/tracing_agent.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ namespace node {
1010
class Environment;
1111

1212
namespace inspector {
13+
class MainThreadHandle;
14+
1315
namespace protocol {
1416

1517
A3E2 class TracingAgent : public NodeTracing::Backend {
1618
public:
17-
explicit TracingAgent(Environment*);
19+
explicit TracingAgent(Environment*, std::shared_ptr<MainThreadHandle>);
1820
~TracingAgent() override;
1921

2022
void Wire(UberDispatcher* dispatcher);
@@ -29,8 +31,10 @@ class TracingAgent : public NodeTracing::Backend {
2931
void DisconnectTraceClient();
3032

3133
Environment* env_;
34+
std::shared_ptr<MainThreadHandle> main_thread_;
3235
tracing::AgentWriterHandle trace_writer_;
33-
std::unique_ptr<NodeTracing::Frontend> frontend_;
36+
int frontend_object_id_;
37+
std::shared_ptr<NodeTracing::Frontend> frontend_;
3438
};
3539

3640

‎src/inspector_agent.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,15 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
211211
const std::unique_ptr<V8Inspector>& inspector,
212212
std::shared_ptr<WorkerManager> worker_manager,
213213
std::unique_ptr<InspectorSessionDelegate> delegate,
214+
std::shared_ptr<MainThreadHandle> main_thread_,
214215
bool prevent_shutdown)
215-
: delegate_(std::move(delegate)),
216-
prevent_shutdown_(prevent_shutdown) {
216+
: delegate_(std::move(delegate)), prevent_shutdown_(prevent_shutdown) {
217217
session_ = inspector->connect(1, this, StringView());
218-
node_dispatcher_.reset(new protocol::UberDispatcher(this));
219-
tracing_agent_.reset(new protocol::TracingAgent(env));
218+
node_dispatcher_ = std::make_unique<protocol::UberDispatcher>(this);
219+
tracing_agent_ =
220+
std::make_unique<protocol::TracingAgent>(env, main_thread_);
220221
tracing_agent_->Wire(node_dispatcher_.get());
221-
worker_agent_.reset(new protocol::WorkerAgent(worker_manager));
222+
worker_agent_ = std::make_unique<protocol::WorkerAgent>(worker_manager);
222223
worker_agent_->Wire(node_dispatcher_.get());
223224
}
224225

@@ -467,9 +468,12 @@ class NodeInspectorClient : public V8InspectorClient {
467468
bool prevent_shutdown) {
468469
events_dispatched_ = true;
469470
int session_id = next_session_id_++;
470-
channels_[session_id] =
471-
std::make_unique<ChannelImpl>(env_, client_, getWorkerManager(),
472-
std::move(delegate), prevent_shutdown);
471+
channels_[session_id] = std::make_unique<ChannelImpl>(env_,
472+
client_,
473+
getWorkerManager(),
474+
std::move(delegate),
475+
getThreadHandle(),
476+
prevent_shutdown);
473477
return session_id;
474478
}
475479

‎test/parallel/test-trace-events-dynamic-enable-workers-disabled.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,4 @@ session.post('NodeTracing.start', {
2525
'Tracing properties can only be changed through main thread sessions'
2626
});
2727
}));
28+
session.disconnect();

0 commit comments

Comments
 (0)
0