Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 808f37c

Browse filesBrowse files
ofrobotsMylesBorins
ofrobots
authored and
MylesBorins
committed
trace_events: destroy platform before tracing
For safer shutdown, we should destroy the platform – and background threads - before the tracing infrastructure is destroyed. This change fixes the relative order of NodePlatform disposition and the tracing agent shutting down. This matches the nesting order for startup. Make the tracing agent own the tracing controller instead of platform to match the above. Fixes: #22865 PR-URL: #22938 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 264d129 commit 808f37c
Copy full SHA for 808f37c

File tree

Expand file treeCollapse file tree

5 files changed

+18
-14
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+18
-14
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,18 @@ static struct {
279279
controller->AddTraceStateObserver(new NodeTraceStateObserver(controller));
280280
tracing::TraceEventHelper::SetTracingController(controller);
281281
StartTracingAgent();
282+
// Tracing must be initialized before platform threads are created.
282283
platform_ = new NodePlatform(thread_pool_size, controller);
283284
V8::InitializePlatform(platform_);
284285
}
285286

286287
void Dispose() {
287-
tracing_agent_.reset(nullptr);
288288
platform_->Shutdown();
289289
delete platform_;
290290
platform_ = nullptr;
291+
// Destroy tracing after the platform (and platform threads) have been
292+
// stopped.
293+
tracing_agent_.reset(nullptr);
291294
}
292295

293296
void DrainVMTasks(Isolate* isolate) {
Collapse file

‎src/node_platform.cc‎

Copy file name to clipboardExpand all lines: src/node_platform.cc
+3-4Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,9 @@ int PerIsolatePlatformData::unref() {
248248
NodePlatform::NodePlatform(int thread_pool_size,
249249
TracingController* tracing_controller) {
250250
if (tracing_controller) {
251-
tracing_controller_.reset(tracing_controller);
251+
tracing_controller_ = tracing_controller;
252252
} else {
253-
TracingController* controller = new TracingController();
254-
tracing_controller_.reset(controller);
253+
tracing_controller_ = new TracingController();
255254
}
256255
background_task_runner_ =
257256
std::make_shared<BackgroundTaskRunner>(thread_pool_size);
@@ -425,7 +424,7 @@ double NodePlatform::CurrentClockTimeMillis() {
425424
}
426425

427426
TracingController* NodePlatform::GetTracingController() {
428-
return tracing_controller_.get();
427+
return tracing_controller_;
429428
}
430429

431430
template <class T>
Collapse file

‎src/node_platform.h‎

Copy file name to clipboardExpand all lines: src/node_platform.h
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ class NodePlatform : public MultiIsolatePlatform {
156156
std::unordered_map<v8::Isolate*,
157157
std::shared_ptr<PerIsolatePlatformData>> per_isolate_;
158158

159-
std::unique_ptr<v8::TracingController> tracing_controller_;
159+
160+
v8::TracingController* tracing_controller_;
160161
std::shared_ptr<BackgroundTaskRunner> background_task_runner_;
161162
};
162163

Collapse file

‎src/tracing/agent.cc‎

Copy file name to clipboardExpand all lines: src/tracing/agent.cc
+5-6Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ using v8::platform::tracing::TraceConfig;
4848
using v8::platform::tracing::TraceWriter;
4949
using std::string;
5050

51-
Agent::Agent() {
52-
tracing_controller_ = new TracingController();
51+
Agent::Agent() : tracing_controller_(new TracingController()) {
5352
tracing_controller_->Initialize(nullptr);
5453

5554
CHECK_EQ(uv_loop_init(&tracing_loop_), 0);
@@ -117,7 +116,7 @@ AgentWriterHandle Agent::AddClient(
117116
use_categories = &categories_with_default;
118117
}
119118

120-
ScopedSuspendTracing suspend(tracing_controller_, this);
119+
ScopedSuspendTracing suspend(tracing_controller_.get(), this);
121120
int id = next_writer_id_++;
122121
AsyncTraceWriter* raw = writer.get();
123122
writers_[id] = std::move(writer);
@@ -157,7 +156,7 @@ void Agent::Disconnect(int client) {
157156
Mutex::ScopedLock lock(initialize_writer_mutex_);
158157
to_be_initialized_.erase(writers_[client].get());
159158
}
160-
ScopedSuspendTracing suspend(tracing_controller_, this);
159+
ScopedSuspendTracing suspend(tracing_controller_.get(), this);
161160
writers_.erase(client);
162161
categories_.erase(client);
163162
}
@@ -166,13 +165,13 @@ void Agent::Enable(int id, const std::set<std::string>& categories) {
166165
if (categories.empty())
167166
return;
168167

169-
ScopedSuspendTracing suspend(tracing_controller_, this,
168+
ScopedSuspendTracing suspend(tracing_controller_.get(), this,
170169
id != kDefaultHandleId);
171170
categories_[id].insert(categories.begin(), categories.end());
172171
}
173172

174173
void Agent::Disable(int id, const std::set<std::string>& categories) {
175-
ScopedSuspendTracing suspend(tracing_controller_, this,
174+
ScopedSuspendTracing suspend(tracing_controller_.get(), this,
176175
id != kDefaultHandleId);
177176
std::multiset<std::string>& writer_categories = categories_[id];
178177
for (const std::string& category : categories) {
Collapse file

‎src/tracing/agent.h‎

Copy file name to clipboardExpand all lines: src/tracing/agent.h
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ class Agent {
6868
Agent();
6969
~Agent();
7070

71-
TracingController* GetTracingController() { return tracing_controller_; }
71+
TracingController* GetTracingController() {
72+
return tracing_controller_.get();
73+
}
7274

7375
enum UseDefaultCategoryMode {
7476
kUseDefaultCategories,
@@ -119,7 +121,7 @@ class Agent {
119121
// These maps store the original arguments to AddClient(), by id.
120122
std::unordered_map<int, std::multiset<std::string>> categories_;
121123
std::unordered_map<int, std::unique_ptr<AsyncTraceWriter>> writers_;
122-
TracingController* tracing_controller_ = nullptr;
124+
std::unique_ptr<TracingController> tracing_controller_;
123125

124126
// Variables related to initializing per-event-loop properties of individual
125127
// writers, such as libuv handles.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.