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: https://github.com/nodejs/node/issues/22865

PR-URL: https://github.com/nodejs/node/pull/22938
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

2 files changed, 9 insertions, 8 deletions

diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc
index 5a4d637bda..fd5cb3387b 100644
--- a/src/tracing/agent.cc
+++ b/src/tracing/agent.cc
@@ -48,8 +48,7 @@ using v8::platform::tracing::TraceConfig;
 using v8::platform::tracing::TraceWriter;
 using std::string;
 
-Agent::Agent() {
-  tracing_controller_ = new TracingController();
+Agent::Agent() : tracing_controller_(new TracingController()) {
   tracing_controller_->Initialize(nullptr);
 
   CHECK_EQ(uv_loop_init(&tracing_loop_), 0);
@@ -117,7 +116,7 @@ AgentWriterHandle Agent::AddClient(
     use_categories = &categories_with_default;
   }
 
-  ScopedSuspendTracing suspend(tracing_controller_, this);
+  ScopedSuspendTracing suspend(tracing_controller_.get(), this);
   int id = next_writer_id_++;
   AsyncTraceWriter* raw = writer.get();
   writers_[id] = std::move(writer);
@@ -157,7 +156,7 @@ void Agent::Disconnect(int client) {
     Mutex::ScopedLock lock(initialize_writer_mutex_);
     to_be_initialized_.erase(writers_[client].get());
   }
-  ScopedSuspendTracing suspend(tracing_controller_, this);
+  ScopedSuspendTracing suspend(tracing_controller_.get(), this);
   writers_.erase(client);
   categories_.erase(client);
 }
@@ -166,13 +165,13 @@ void Agent::Enable(int id, const std::set<std::string>& categories) {
   if (categories.empty())
     return;
 
-  ScopedSuspendTracing suspend(tracing_controller_, this,
+  ScopedSuspendTracing suspend(tracing_controller_.get(), this,
                                id != kDefaultHandleId);
   categories_[id].insert(categories.begin(), categories.end());
 }
 
 void Agent::Disable(int id, const std::set<std::string>& categories) {
-  ScopedSuspendTracing suspend(tracing_controller_, this,
+  ScopedSuspendTracing suspend(tracing_controller_.get(), this,
                                id != kDefaultHandleId);
   std::multiset<std::string>& writer_categories = categories_[id];
   for (const std::string& category : categories) {
diff --git a/src/tracing/agent.h b/src/tracing/agent.h
index e79480a036..e9f52abe64 100644
--- a/src/tracing/agent.h
+++ b/src/tracing/agent.h
@@ -70,7 +70,9 @@ class Agent {
   Agent();
   ~Agent();
 
-  TracingController* GetTracingController() { return tracing_controller_; }
+  TracingController* GetTracingController() {
+    return tracing_controller_.get();
+  }
 
   enum UseDefaultCategoryMode {
     kUseDefaultCategories,
@@ -121,7 +123,7 @@ class Agent {
   // These maps store the original arguments to AddClient(), by id.
   std::unordered_map<int, std::multiset<std::string>> categories_;
   std::unordered_map<int, std::unique_ptr<AsyncTraceWriter>> writers_;
-  TracingController* tracing_controller_ = nullptr;
+  std::unique_ptr<TracingController> tracing_controller_;
 
   // Variables related to initializing per-event-loop properties of individual
   // writers, such as libuv handles.