diff --git a/src/env-inl.h b/src/env-inl.h index a294066bb5aa0d..c38e37f786374d 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -32,7 +32,6 @@ #include "v8.h" #include "node_perf_common.h" #include "node_context_data.h" -#include "tracing/agent.h" #include "node_worker.h" #include @@ -326,8 +325,8 @@ inline v8::Isolate* Environment::isolate() const { return isolate_; } -inline tracing::Agent* Environment::tracing_agent() const { - return tracing_agent_; +inline tracing::AgentWriterHandle* Environment::tracing_agent_writer() const { + return tracing_agent_writer_; } inline Environment* Environment::from_timer_handle(uv_timer_t* handle) { diff --git a/src/env.cc b/src/env.cc index 769676275b25f0..244c6d8be37685 100644 --- a/src/env.cc +++ b/src/env.cc @@ -105,10 +105,10 @@ void InitThreadLocalOnce() { Environment::Environment(IsolateData* isolate_data, Local context, - tracing::Agent* tracing_agent) + tracing::AgentWriterHandle* tracing_agent_writer) : isolate_(context->GetIsolate()), isolate_data_(isolate_data), - tracing_agent_(tracing_agent), + tracing_agent_writer_(tracing_agent_writer), immediate_info_(context->GetIsolate()), tick_info_(context->GetIsolate()), timer_base_(uv_now(isolate_data->event_loop())), diff --git a/src/env.h b/src/env.h index acbdd013285d6c..91ae7824281cb7 100644 --- a/src/env.h +++ b/src/env.h @@ -35,7 +35,6 @@ #include "v8.h" #include "node.h" #include "node_http2_state.h" -#include "tracing/agent.h" #include #include @@ -55,6 +54,10 @@ namespace performance { class performance_state; } +namespace tracing { +class AgentWriterHandle; +} + namespace worker { class Worker; } @@ -584,7 +587,7 @@ class Environment { Environment(IsolateData* isolate_data, v8::Local context, - tracing::Agent* tracing_agent); + tracing::AgentWriterHandle* tracing_agent_writer); ~Environment(); void Start(int argc, @@ -622,7 +625,7 @@ class Environment { inline bool profiler_idle_notifier_started() const; inline v8::Isolate* isolate() const; - inline tracing::Agent* tracing_agent() const; + inline tracing::AgentWriterHandle* tracing_agent_writer() const; inline uv_loop_t* event_loop() const; inline uint32_t watched_providers() const; @@ -876,7 +879,7 @@ class Environment { v8::Isolate* const isolate_; IsolateData* const isolate_data_; - tracing::Agent* const tracing_agent_; + tracing::AgentWriterHandle* const tracing_agent_writer_; uv_timer_t timer_handle_; uv_check_t immediate_check_handle_; uv_idle_t immediate_idle_handle_; diff --git a/src/inspector/tracing_agent.cc b/src/inspector/tracing_agent.cc index c0425aab29d4a0..6e962b289ab36f 100644 --- a/src/inspector/tracing_agent.cc +++ b/src/inspector/tracing_agent.cc @@ -46,9 +46,7 @@ class InspectorTraceWriter : public node::tracing::AsyncTraceWriter { } // namespace TracingAgent::TracingAgent(Environment* env) - : env_(env), - trace_writer_( - tracing::Agent::EmptyClientHandle()) { + : env_(env) { } TracingAgent::~TracingAgent() { @@ -62,7 +60,7 @@ void TracingAgent::Wire(UberDispatcher* dispatcher) { DispatchResponse TracingAgent::start( std::unique_ptr traceConfig) { - if (trace_writer_ != nullptr) { + if (!trace_writer_.empty()) { return DispatchResponse::Error( "Call NodeTracing::end to stop tracing before updating the config"); } @@ -76,10 +74,11 @@ DispatchResponse TracingAgent::start( if (categories_set.empty()) return DispatchResponse::Error("At least one category should be enabled"); - trace_writer_ = env_->tracing_agent()->AddClient( + trace_writer_ = env_->tracing_agent_writer()->agent()->AddClient( categories_set, std::unique_ptr( - new InspectorTraceWriter(frontend_.get()))); + new InspectorTraceWriter(frontend_.get())), + tracing::Agent::kIgnoreDefaultCategories); return DispatchResponse::OK(); } diff --git a/src/inspector/tracing_agent.h b/src/inspector/tracing_agent.h index 478107c5acdd64..029fce7c191b42 100644 --- a/src/inspector/tracing_agent.h +++ b/src/inspector/tracing_agent.h @@ -2,16 +2,13 @@ #define SRC_INSPECTOR_TRACING_AGENT_H_ #include "node/inspector/protocol/NodeTracing.h" +#include "tracing/agent.h" #include "v8.h" namespace node { class Environment; -namespace tracing { -class Agent; -} // namespace tracing - namespace inspector { namespace protocol { @@ -32,8 +29,7 @@ class TracingAgent : public NodeTracing::Backend { void DisconnectTraceClient(); Environment* env_; - std::unique_ptr, - void (*)(std::pair*)> trace_writer_; + tracing::AgentWriterHandle trace_writer_; std::unique_ptr frontend_; }; diff --git a/src/node.cc b/src/node.cc index c4fc05f75fe2b1..886243fd5cdb50 100644 --- a/src/node.cc +++ b/src/node.cc @@ -61,6 +61,7 @@ #include "req_wrap-inl.h" #include "string_bytes.h" #include "tracing/agent.h" +#include "tracing/node_trace_writer.h" #include "util.h" #include "uv.h" #if NODE_USE_V8_PLATFORM @@ -387,7 +388,7 @@ class NodeTraceStateObserver : static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) { - tracing_agent_.reset(new tracing::Agent(trace_file_pattern)); + tracing_agent_.reset(new tracing::Agent()); auto controller = tracing_agent_->GetTracingController(); controller->AddTraceStateObserver(new NodeTraceStateObserver(controller)); tracing::TraceEventHelper::SetTracingController(controller); @@ -397,10 +398,10 @@ static struct { } void Dispose() { + tracing_agent_.reset(nullptr); platform_->Shutdown(); delete platform_; platform_ = nullptr; - tracing_agent_.reset(nullptr); } void DrainVMTasks(Isolate* isolate) { @@ -427,16 +428,23 @@ static struct { #endif // HAVE_INSPECTOR void StartTracingAgent() { - tracing_agent_->Enable(trace_enabled_categories); + if (trace_enabled_categories.empty()) { + tracing_file_writer_ = tracing_agent_->DefaultHandle(); + } else { + tracing_file_writer_ = tracing_agent_->AddClient( + ParseCommaSeparatedSet(trace_enabled_categories), + std::unique_ptr( + new tracing::NodeTraceWriter(trace_file_pattern)), + tracing::Agent::kUseDefaultCategories); + } } void StopTracingAgent() { - if (tracing_agent_) - tracing_agent_->Stop(); + tracing_file_writer_.reset(); } - tracing::Agent* GetTracingAgent() const { - return tracing_agent_.get(); + tracing::AgentWriterHandle* GetTracingAgentWriter() { + return &tracing_file_writer_; } NodePlatform* Platform() { @@ -444,6 +452,7 @@ static struct { } std::unique_ptr tracing_agent_; + tracing::AgentWriterHandle tracing_file_writer_; NodePlatform* platform_; #else // !NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) {} @@ -464,7 +473,9 @@ static struct { } void StopTracingAgent() {} - tracing::Agent* GetTracingAgent() const { return nullptr; } + tracing::AgentWriterHandle* GetTracingAgentWriter() { + return nullptr; + } NodePlatform* Platform() { return nullptr; @@ -3588,7 +3599,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data, HandleScope handle_scope(isolate); Context::Scope context_scope(context); auto env = new Environment(isolate_data, context, - v8_platform.GetTracingAgent()); + v8_platform.GetTracingAgentWriter()); env->Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); return env; } @@ -3647,7 +3658,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, HandleScope handle_scope(isolate); Local context = NewContext(isolate); Context::Scope context_scope(context); - Environment env(isolate_data, context, v8_platform.GetTracingAgent()); + Environment env(isolate_data, context, v8_platform.GetTracingAgentWriter()); env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); const char* path = argc > 1 ? argv[1] : nullptr; diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc index 684ea41a66cc58..d59b92555795fb 100644 --- a/src/node_trace_events.cc +++ b/src/node_trace_events.cc @@ -25,7 +25,7 @@ class NodeCategorySet : public BaseObject { static void Enable(const FunctionCallbackInfo& args); static void Disable(const FunctionCallbackInfo& args); - const std::set& GetCategories() { return categories_; } + const std::set& GetCategories() const { return categories_; } void MemoryInfo(MemoryTracker* tracker) const override { tracker->TrackThis(this); @@ -37,8 +37,8 @@ class NodeCategorySet : public BaseObject { private: NodeCategorySet(Environment* env, Local wrap, - std::set categories) : - BaseObject(env, wrap), categories_(categories) { + std::set&& categories) : + BaseObject(env, wrap), categories_(std::move(categories)) { MakeWeak(); } @@ -52,12 +52,14 @@ void NodeCategorySet::New(const FunctionCallbackInfo& args) { CHECK(args[0]->IsArray()); Local cats = args[0].As(); for (size_t n = 0; n < cats->Length(); n++) { - Local category = cats->Get(env->context(), n).ToLocalChecked(); + Local category; + if (!cats->Get(env->context(), n).ToLocal(&category)) return; Utf8Value val(env->isolate(), category); + if (!*val) return; categories.emplace(*val); } - CHECK_NOT_NULL(env->tracing_agent()); - new NodeCategorySet(env, args.This(), categories); + CHECK_NOT_NULL(env->tracing_agent_writer()); + new NodeCategorySet(env, args.This(), std::move(categories)); } void NodeCategorySet::Enable(const FunctionCallbackInfo& args) { @@ -67,7 +69,7 @@ void NodeCategorySet::Enable(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(category_set); const auto& categories = category_set->GetCategories(); if (!category_set->enabled_ && !categories.empty()) { - env->tracing_agent()->Enable(categories); + env->tracing_agent_writer()->Enable(categories); category_set->enabled_ = true; } } @@ -79,25 +81,28 @@ void NodeCategorySet::Disable(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(category_set); const auto& categories = category_set->GetCategories(); if (category_set->enabled_ && !categories.empty()) { - env->tracing_agent()->Disable(categories); + env->tracing_agent_writer()->Disable(categories); category_set->enabled_ = false; } } void GetEnabledCategories(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - std::string categories = env->tracing_agent()->GetEnabledCategories(); + std::string categories = + env->tracing_agent_writer()->agent()->GetEnabledCategories(); if (!categories.empty()) { args.GetReturnValue().Set( String::NewFromUtf8(env->isolate(), categories.c_str(), - v8::NewStringType::kNormal).ToLocalChecked()); + v8::NewStringType::kNormal, + categories.size()).ToLocalChecked()); } } // The tracing APIs require category groups to be pointers to long-lived // strings. Those strings are stored here. -static std::unordered_set categoryGroups; +static std::unordered_set category_groups; +static Mutex category_groups_mutex; // Gets a pointer to the category-enabled flags for a tracing category group, // if tracing is enabled for it. @@ -107,14 +112,15 @@ static const uint8_t* GetCategoryGroupEnabled(const char* category_group) { } static const char* GetCategoryGroup(Environment* env, - const Local categoryValue) { - CHECK(categoryValue->IsString()); + const Local category_value) { + CHECK(category_value->IsString()); - Utf8Value category(env->isolate(), categoryValue); + Utf8Value category(env->isolate(), category_value); + Mutex::ScopedLock lock(category_groups_mutex); // If the value already exists in the set, insertion.first will point // to the existing value. Thus, this will maintain a long lived pointer // to the category c-string. - auto insertion = categoryGroups.insert(category.out()); + auto insertion = category_groups.insert(category.out()); // The returned insertion is a pair whose first item is the object that was // inserted or that blocked the insertion and second item is a boolean @@ -133,7 +139,7 @@ static void Emit(const FunctionCallbackInfo& args) { // enabled. const char* category_group = GetCategoryGroup(env, args[1]); const uint8_t* category_group_enabled = - GetCategoryGroupEnabled(category_group); + GetCategoryGroupEnabled(category_group); if (*category_group_enabled == 0) return; // get trace_event phase @@ -142,8 +148,8 @@ static void Emit(const FunctionCallbackInfo& args) { // get trace_event name CHECK(args[2]->IsString()); - Utf8Value nameValue(env->isolate(), args[2]); - const char* name = nameValue.out(); + Utf8Value name_value(env->isolate(), args[2]); + const char* name = name_value.out(); // get trace_event id int64_t id = 0; @@ -212,7 +218,7 @@ static void CategoryGroupEnabled(const FunctionCallbackInfo& args) { const char* category_group = GetCategoryGroup(env, args[0]); const uint8_t* category_group_enabled = - GetCategoryGroupEnabled(category_group); + GetCategoryGroupEnabled(category_group); args.GetReturnValue().Set(*category_group_enabled > 0); } diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index a3ddfb61a95328..5a4d637bda0356 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -1,23 +1,26 @@ #include "tracing/agent.h" -#include #include #include "tracing/node_trace_buffer.h" -#include "tracing/node_trace_writer.h" +#include "debug_utils.h" +#include "env-inl.h" namespace node { namespace tracing { -namespace { - -class ScopedSuspendTracing { +class Agent::ScopedSuspendTracing { public: - ScopedSuspendTracing(TracingController* controller, Agent* agent) - : controller_(controller), agent_(agent) { - controller->StopTracing(); + ScopedSuspendTracing(TracingController* controller, Agent* agent, + bool do_suspend = true) + : controller_(controller), agent_(do_suspend ? agent : nullptr) { + if (do_suspend) { + CHECK(agent_->started_); + controller->StopTracing(); + } } ~ScopedSuspendTracing() { + if (agent_ == nullptr) return; TraceConfig* config = agent_->CreateTraceConfig(); if (config != nullptr) { controller_->StartTracing(config); @@ -29,8 +32,10 @@ class ScopedSuspendTracing { Agent* agent_; }; +namespace { + std::set flatten( - const std::unordered_map>& map) { + const std::unordered_map>& map) { std::set result; for (const auto& id_value : map) result.insert(id_value.second.begin(), id_value.second.end()); @@ -43,18 +48,45 @@ using v8::platform::tracing::TraceConfig; using v8::platform::tracing::TraceWriter; using std::string; -Agent::Agent(const std::string& log_file_pattern) - : log_file_pattern_(log_file_pattern), file_writer_(EmptyClientHandle()) { +Agent::Agent() { tracing_controller_ = new TracingController(); tracing_controller_->Initialize(nullptr); + + CHECK_EQ(uv_loop_init(&tracing_loop_), 0); + CHECK_EQ(uv_async_init(&tracing_loop_, + &initialize_writer_async_, + [](uv_async_t* async) { + Agent* agent = ContainerOf(&Agent::initialize_writer_async_, async); + agent->InitializeWritersOnThread(); + }), 0); + uv_unref(reinterpret_cast(&initialize_writer_async_)); +} + +void Agent::InitializeWritersOnThread() { + Mutex::ScopedLock lock(initialize_writer_mutex_); + while (!to_be_initialized_.empty()) { + AsyncTraceWriter* head = *to_be_initialized_.begin(); + head->InitializeOnThread(&tracing_loop_); + to_be_initialized_.erase(head); + } + initialize_writer_condvar_.Broadcast(lock); +} + +Agent::~Agent() { + categories_.clear(); + writers_.clear(); + + StopTracing(); + + uv_close(reinterpret_cast(&initialize_writer_async_), nullptr); + uv_run(&tracing_loop_, UV_RUN_ONCE); + CheckedUvLoopClose(&tracing_loop_); } void Agent::Start() { if (started_) return; - CHECK_EQ(uv_loop_init(&tracing_loop_), 0); - NodeTraceBuffer* trace_buffer_ = new NodeTraceBuffer( NodeTraceBuffer::kBufferChunks, this, &tracing_loop_); tracing_controller_->Initialize(trace_buffer_); @@ -62,24 +94,48 @@ void Agent::Start() { // This thread should be created *after* async handles are created // (within NodeTraceWriter and NodeTraceBuffer constructors). // Otherwise the thread could shut down prematurely. - CHECK_EQ(0, uv_thread_create(&thread_, ThreadCb, this)); + CHECK_EQ(0, uv_thread_create(&thread_, [](void* arg) { + Agent* agent = static_cast(arg); + uv_run(&agent->tracing_loop_, UV_RUN_DEFAULT); + }, this)); started_ = true; } -Agent::ClientHandle Agent::AddClient(const std::set& categories, - std::unique_ptr writer) { +AgentWriterHandle Agent::AddClient( + const std::set& categories, + std::unique_ptr writer, + enum UseDefaultCategoryMode mode) { Start(); + + const std::set* use_categories = &categories; + + std::set categories_with_default; + if (mode == kUseDefaultCategories) { + categories_with_default.insert(categories.begin(), categories.end()); + categories_with_default.insert(categories_[kDefaultHandleId].begin(), + categories_[kDefaultHandleId].end()); + use_categories = &categories_with_default; + } + ScopedSuspendTracing suspend(tracing_controller_, this); int id = next_writer_id_++; + AsyncTraceWriter* raw = writer.get(); writers_[id] = std::move(writer); - categories_[id] = categories; + categories_[id] = { use_categories->begin(), use_categories->end() }; + + { + Mutex::ScopedLock lock(initialize_writer_mutex_); + to_be_initialized_.insert(raw); + uv_async_send(&initialize_writer_async_); + while (to_be_initialized_.count(raw) > 0) + initialize_writer_condvar_.Wait(lock); + } - auto client_id = new std::pair(this, id); - return ClientHandle(client_id, &DisconnectClient); + return AgentWriterHandle(this, id); } -void Agent::Stop() { - file_writer_.reset(); +AgentWriterHandle Agent::DefaultHandle() { + return AgentWriterHandle(this, kDefaultHandleId); } void Agent::StopTracing() { @@ -96,66 +152,37 @@ void Agent::StopTracing() { } void Agent::Disconnect(int client) { + if (client == kDefaultHandleId) return; + { + Mutex::ScopedLock lock(initialize_writer_mutex_); + to_be_initialized_.erase(writers_[client].get()); + } ScopedSuspendTracing suspend(tracing_controller_, this); writers_.erase(client); categories_.erase(client); } -// static -void Agent::ThreadCb(void* arg) { - Agent* agent = static_cast(arg); - uv_run(&agent->tracing_loop_, UV_RUN_DEFAULT); -} - -void Agent::Enable(const std::string& categories) { +void Agent::Enable(int id, const std::set& categories) { if (categories.empty()) return; - std::set categories_set; - std::stringstream category_list(categories); - while (category_list.good()) { - std::string category; - getline(category_list, category, ','); - categories_set.insert(category); - } - Enable(categories_set); -} -void Agent::Enable(const std::set& categories) { - std::string cats; - for (const std::string cat : categories) - cats += cat + ", "; - if (categories.empty()) - return; - - file_writer_categories_.insert(categories.begin(), categories.end()); - std::set full_list(file_writer_categories_.begin(), - file_writer_categories_.end()); - if (!file_writer_) { - // Ensure background thread is running - Start(); - std::unique_ptr writer( - new NodeTraceWriter(log_file_pattern_, &tracing_loop_)); - file_writer_ = AddClient(full_list, std::move(writer)); - } else { - ScopedSuspendTracing suspend(tracing_controller_, this); - categories_[file_writer_->second] = full_list; - } + ScopedSuspendTracing suspend(tracing_controller_, this, + id != kDefaultHandleId); + categories_[id].insert(categories.begin(), categories.end()); } -void Agent::Disable(const std::set& categories) { - for (auto category : categories) { - auto it = file_writer_categories_.find(category); - if (it != file_writer_categories_.end()) - file_writer_categories_.erase(it); +void Agent::Disable(int id, const std::set& categories) { + ScopedSuspendTracing suspend(tracing_controller_, this, + id != kDefaultHandleId); + std::multiset& writer_categories = categories_[id]; + for (const std::string& category : categories) { + auto it = writer_categories.find(category); + if (it != writer_categories.end()) + writer_categories.erase(it); } - if (!file_writer_) - return; - ScopedSuspendTracing suspend(tracing_controller_, this); - categories_[file_writer_->second] = { file_writer_categories_.begin(), - file_writer_categories_.end() }; } -TraceConfig* Agent::CreateTraceConfig() { +TraceConfig* Agent::CreateTraceConfig() const { if (categories_.empty()) return nullptr; TraceConfig* trace_config = new TraceConfig(); @@ -165,9 +192,9 @@ TraceConfig* Agent::CreateTraceConfig() { return trace_config; } -std::string Agent::GetEnabledCategories() { +std::string Agent::GetEnabledCategories() const { std::string categories; - for (const auto& category : flatten(categories_)) { + for (const std::string& category : flatten(categories_)) { if (!categories.empty()) categories += ','; categories += category; @@ -184,5 +211,6 @@ void Agent::Flush(bool blocking) { for (const auto& id_writer : writers_) id_writer.second->Flush(blocking); } + } // namespace tracing } // namespace node diff --git a/src/tracing/agent.h b/src/tracing/agent.h index fd7984275969ba..045aaef85e8ea6 100644 --- a/src/tracing/agent.h +++ b/src/tracing/agent.h @@ -4,6 +4,8 @@ #include "libplatform/v8-tracing.h" #include "uv.h" #include "v8.h" +#include "util.h" +#include "node_mutex.h" #include #include @@ -15,11 +17,14 @@ namespace tracing { using v8::platform::tracing::TraceConfig; using v8::platform::tracing::TraceObject; +class Agent; + class AsyncTraceWriter { public: virtual ~AsyncTraceWriter() {} virtual void AppendTraceEvent(TraceObject* trace_event) = 0; virtual void Flush(bool blocking) = 0; + virtual void InitializeOnThread(uv_loop_t* loop) {} }; class TracingController : public v8::platform::tracing::TracingController { @@ -31,60 +36,125 @@ class TracingController : public v8::platform::tracing::TracingController { } }; +class AgentWriterHandle { + public: + inline AgentWriterHandle() {} + inline ~AgentWriterHandle() { reset(); } + + inline AgentWriterHandle(AgentWriterHandle&& other); + inline AgentWriterHandle& operator=(AgentWriterHandle&& other); + inline bool empty() const { return agent_ == nullptr; } + inline void reset(); + + inline void Enable(const std::set& categories); + inline void Disable(const std::set& categories); + + inline Agent* agent() { return agent_; } + + private: + inline AgentWriterHandle(Agent* agent, int id) : agent_(agent), id_(id) {} + + AgentWriterHandle(const AgentWriterHandle& other) = delete; + AgentWriterHandle& operator=(const AgentWriterHandle& other) = delete; + + Agent* agent_ = nullptr; + int id_; + + friend class Agent; +}; class Agent { public: - // Resetting the pointer disconnects client - using ClientHandle = std::unique_ptr, - void (*)(std::pair*)>; - - static ClientHandle EmptyClientHandle() { - return ClientHandle(nullptr, DisconnectClient); - } - explicit Agent(const std::string& log_file_pattern); - void Stop(); + Agent(); + ~Agent(); TracingController* GetTracingController() { return tracing_controller_; } - // Destroying the handle disconnects the client - ClientHandle AddClient(const std::set& categories, - std::unique_ptr writer); - - // These 3 methods operate on a "default" client, e.g. the file writer - void Enable(const std::string& categories); - void Enable(const std::set& categories); - void Disable(const std::set& categories); - std::string GetEnabledCategories(); + enum UseDefaultCategoryMode { + kUseDefaultCategories, + kIgnoreDefaultCategories + }; + // Destroying the handle disconnects the client + AgentWriterHandle AddClient(const std::set& categories, + std::unique_ptr writer, + enum UseDefaultCategoryMode mode); + // A handle that is only used for managing the default categories + // (which can then implicitly be used through using `USE_DEFAULT_CATEGORIES` + // when adding a client later). + AgentWriterHandle DefaultHandle(); + + // Returns a comma-separated list of enabled categories. + std::string GetEnabledCategories() const; + + // Writes to all writers registered through AddClient(). void AppendTraceEvent(TraceObject* trace_event); + // Flushes all writers registered through AddClient(). void Flush(bool blocking); - TraceConfig* CreateTraceConfig(); + TraceConfig* CreateTraceConfig() const; private: + friend class AgentWriterHandle; + static void ThreadCb(void* arg); - static void DisconnectClient(std::pair* id_agent) { - id_agent->first->Disconnect(id_agent->second); - delete id_agent; - } + void InitializeWritersOnThread(); void Start(); void StopTracing(); void Disconnect(int client); - const std::string& log_file_pattern_; + void Enable(int id, const std::set& categories); + void Disable(int id, const std::set& categories); + uv_thread_t thread_; uv_loop_t tracing_loop_; + bool started_ = false; + class ScopedSuspendTracing; - std::unordered_map> categories_; - TracingController* tracing_controller_ = nullptr; - ClientHandle file_writer_; + // Each individual Writer has one id. int next_writer_id_ = 1; + enum { kDefaultHandleId = -1 }; + // These maps store the original arguments to AddClient(), by id. + std::unordered_map> categories_; std::unordered_map> writers_; - std::multiset file_writer_categories_; + TracingController* tracing_controller_ = nullptr; + + // Variables related to initializing per-event-loop properties of individual + // writers, such as libuv handles. + Mutex initialize_writer_mutex_; + ConditionVariable initialize_writer_condvar_; + uv_async_t initialize_writer_async_; + std::set to_be_initialized_; }; +void AgentWriterHandle::reset() { + if (agent_ != nullptr) + agent_->Disconnect(id_); + agent_ = nullptr; +} + +AgentWriterHandle& AgentWriterHandle::operator=(AgentWriterHandle&& other) { + reset(); + agent_ = other.agent_; + id_ = other.id_; + other.agent_ = nullptr; + return *this; +} + +AgentWriterHandle::AgentWriterHandle(AgentWriterHandle&& other) { + *this = std::move(other); +} + +void AgentWriterHandle::Enable(const std::set& categories) { + if (agent_ != nullptr) agent_->Enable(id_, categories); +} + +void AgentWriterHandle::Disable(const std::set& categories) { + if (agent_ != nullptr) agent_->Disable(id_, categories); +} + } // namespace tracing } // namespace node diff --git a/src/tracing/node_trace_writer.cc b/src/tracing/node_trace_writer.cc index a2f7bebfc72f9a..a0382e587b3ad7 100644 --- a/src/tracing/node_trace_writer.cc +++ b/src/tracing/node_trace_writer.cc @@ -3,16 +3,25 @@ #include #include -#include "util.h" +#include "util-inl.h" namespace node { namespace tracing { -NodeTraceWriter::NodeTraceWriter(const std::string& log_file_pattern, - uv_loop_t* tracing_loop) - : tracing_loop_(tracing_loop), log_file_pattern_(log_file_pattern) { +NodeTraceWriter::NodeTraceWriter(const std::string& log_file_pattern) + : log_file_pattern_(log_file_pattern) {} + +void NodeTraceWriter::InitializeOnThread(uv_loop_t* loop) { + CHECK_NULL(tracing_loop_); + tracing_loop_ = loop; + flush_signal_.data = this; - int err = uv_async_init(tracing_loop_, &flush_signal_, FlushSignalCb); + int err = uv_async_init(tracing_loop_, &flush_signal_, + [](uv_async_t* signal) { + NodeTraceWriter* trace_writer = + ContainerOf(&NodeTraceWriter::flush_signal_, signal); + trace_writer->FlushPrivate(); + }); CHECK_EQ(err, 0); exit_signal_.data = this; @@ -28,9 +37,7 @@ void NodeTraceWriter::WriteSuffix() { { Mutex::ScopedLock scoped_lock(stream_mutex_); if (total_traces_ > 0) { - total_traces_ = 0; // so we don't write it again in FlushPrivate - // Appends "]}" to stream_. - delete json_trace_writer_; + total_traces_ = kTracesPerFile; // Act as if we reached the file limit. should_flush = true; } } @@ -44,7 +51,7 @@ NodeTraceWriter::~NodeTraceWriter() { uv_fs_t req; int err; if (fd_ != -1) { - err = uv_fs_close(tracing_loop_, &req, fd_, nullptr); + err = uv_fs_close(nullptr, &req, fd_, nullptr); CHECK_EQ(err, 0); uv_fs_req_cleanup(&req); } @@ -75,10 +82,20 @@ void NodeTraceWriter::OpenNewFileForStreaming() { replace_substring(&filepath, "${pid}", std::to_string(uv_os_getpid())); replace_substring(&filepath, "${rotation}", std::to_string(file_num_)); - fd_ = uv_fs_open(tracing_loop_, &req, filepath.c_str(), + if (fd_ != -1) { + CHECK_EQ(uv_fs_close(nullptr, &req, fd_, nullptr), 0); + uv_fs_req_cleanup(&req); + } + + fd_ = uv_fs_open(nullptr, &req, filepath.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0644, nullptr); - CHECK_NE(fd_, -1); uv_fs_req_cleanup(&req); + if (fd_ < 0) { + fprintf(stderr, "Could not open trace file %s: %s\n", + filepath.c_str(), + uv_strerror(fd_)); + fd_ = -1; + } } void NodeTraceWriter::AppendTraceEvent(TraceObject* trace_event) { @@ -92,7 +109,7 @@ void NodeTraceWriter::AppendTraceEvent(TraceObject* trace_event) { // to a state where we can start writing trace events to it. // Repeatedly constructing and destroying json_trace_writer_ allows // us to use V8's JSON writer instead of implementing our own. - json_trace_writer_ = TraceWriter::CreateJSONTraceWriter(stream_); + json_trace_writer_.reset(TraceWriter::CreateJSONTraceWriter(stream_)); } ++total_traces_; json_trace_writer_->AppendTraceEvent(trace_event); @@ -107,7 +124,7 @@ void NodeTraceWriter::FlushPrivate() { total_traces_ = 0; // Destroying the member JSONTraceWriter object appends "]}" to // stream_ - in other words, ending a JSON file. - delete json_trace_writer_; + json_trace_writer_.reset(); } // str() makes a copy of the contents of the stream. str = stream_.str(); @@ -121,11 +138,6 @@ void NodeTraceWriter::FlushPrivate() { WriteToFile(std::move(str), highest_request_id); } -void NodeTraceWriter::FlushSignalCb(uv_async_t* signal) { - NodeTraceWriter* trace_writer = static_cast(signal->data); - trace_writer->FlushPrivate(); -} - void NodeTraceWriter::Flush(bool blocking) { Mutex::ScopedLock scoped_lock(request_mutex_); if (!json_trace_writer_) { @@ -145,49 +157,71 @@ void NodeTraceWriter::Flush(bool blocking) { } void NodeTraceWriter::WriteToFile(std::string&& str, int highest_request_id) { - WriteRequest* write_req = new WriteRequest(); - write_req->str = std::move(str); - write_req->writer = this; - write_req->highest_request_id = highest_request_id; - uv_buf_t uv_buf = uv_buf_init(const_cast(write_req->str.c_str()), - write_req->str.length()); - request_mutex_.Lock(); - // Manage a queue of WriteRequest objects because the behavior of uv_write is - // undefined if the same WriteRequest object is used more than once - // between WriteCb calls. In addition, this allows us to keep track of the id - // of the latest write request that actually been completed. - write_req_queue_.push(write_req); - request_mutex_.Unlock(); - int err = uv_fs_write(tracing_loop_, reinterpret_cast(write_req), - fd_, &uv_buf, 1, -1, WriteCb); + if (fd_ == -1) return; + + uv_buf_t buf = uv_buf_init(nullptr, 0); + { + Mutex::ScopedLock lock(request_mutex_); + write_req_queue_.emplace(WriteRequest { + std::move(str), highest_request_id + }); + if (write_req_queue_.size() == 1) { + buf = uv_buf_init( + const_cast(write_req_queue_.front().str.c_str()), + write_req_queue_.front().str.length()); + } + } + // Only one write request for the same file descriptor should be active at + // a time. + if (buf.base != nullptr && fd_ != -1) { + StartWrite(buf); + } +} + +void NodeTraceWriter::StartWrite(uv_buf_t buf) { + int err = uv_fs_write( + tracing_loop_, &write_req_, fd_, &buf, 1, -1, + [](uv_fs_t* req) { + NodeTraceWriter* writer = + ContainerOf(&NodeTraceWriter::write_req_, req); + writer->AfterWrite(); + }); CHECK_EQ(err, 0); } -void NodeTraceWriter::WriteCb(uv_fs_t* req) { - WriteRequest* write_req = reinterpret_cast(req); - CHECK_GE(write_req->req.result, 0); +void NodeTraceWriter::AfterWrite() { + CHECK_GE(write_req_.result, 0); + uv_fs_req_cleanup(&write_req_); - NodeTraceWriter* writer = write_req->writer; - int highest_request_id = write_req->highest_request_id; + uv_buf_t buf = uv_buf_init(nullptr, 0); { - Mutex::ScopedLock scoped_lock(writer->request_mutex_); - CHECK_EQ(write_req, writer->write_req_queue_.front()); - writer->write_req_queue_.pop(); - writer->highest_request_id_completed_ = highest_request_id; - writer->request_cond_.Broadcast(scoped_lock); + Mutex::ScopedLock scoped_lock(request_mutex_); + int highest_request_id = write_req_queue_.front().highest_request_id; + write_req_queue_.pop(); + highest_request_id_completed_ = highest_request_id; + request_cond_.Broadcast(scoped_lock); + if (!write_req_queue_.empty()) { + buf = uv_buf_init( + const_cast(write_req_queue_.front().str.c_str()), + write_req_queue_.front().str.length()); + } + } + if (buf.base != nullptr && fd_ != -1) { + StartWrite(buf); } - delete write_req; } // static void NodeTraceWriter::ExitSignalCb(uv_async_t* signal) { - NodeTraceWriter* trace_writer = static_cast(signal->data); + NodeTraceWriter* trace_writer = + ContainerOf(&NodeTraceWriter::exit_signal_, signal); uv_close(reinterpret_cast(&trace_writer->flush_signal_), nullptr); uv_close(reinterpret_cast(&trace_writer->exit_signal_), [](uv_handle_t* signal) { NodeTraceWriter* trace_writer = - static_cast(signal->data); + ContainerOf(&NodeTraceWriter::exit_signal_, + reinterpret_cast(signal)); Mutex::ScopedLock scoped_lock(trace_writer->request_mutex_); trace_writer->exited_ = true; trace_writer->exit_cond_.Signal(scoped_lock); diff --git a/src/tracing/node_trace_writer.h b/src/tracing/node_trace_writer.h index b2d5e7912fa01d..5e5781479c689f 100644 --- a/src/tracing/node_trace_writer.h +++ b/src/tracing/node_trace_writer.h @@ -4,7 +4,6 @@ #include #include -#include "node_mutex.h" #include "libplatform/v8-tracing.h" #include "tracing/agent.h" #include "uv.h" @@ -17,10 +16,10 @@ using v8::platform::tracing::TraceWriter; class NodeTraceWriter : public AsyncTraceWriter { public: - explicit NodeTraceWriter(const std::string& log_file_pattern, - uv_loop_t* tracing_loop); + explicit NodeTraceWriter(const std::string& log_file_pattern); ~NodeTraceWriter(); + void InitializeOnThread(uv_loop_t* loop) override; void AppendTraceEvent(TraceObject* trace_event) override; void Flush(bool blocking) override; @@ -28,21 +27,19 @@ class NodeTraceWriter : public AsyncTraceWriter { private: struct WriteRequest { - uv_fs_t req; - NodeTraceWriter* writer; std::string str; int highest_request_id; }; - static void WriteCb(uv_fs_t* req); + void AfterWrite(); + void StartWrite(uv_buf_t buf); void OpenNewFileForStreaming(); void WriteToFile(std::string&& str, int highest_request_id); void WriteSuffix(); - static void FlushSignalCb(uv_async_t* signal); void FlushPrivate(); static void ExitSignalCb(uv_async_t* signal); - uv_loop_t* tracing_loop_; + uv_loop_t* tracing_loop_ = nullptr; // Triggers callback to initiate writing the contents of stream_ to disk. uv_async_t flush_signal_; // Triggers callback to close async objects, ending the tracing thread. @@ -58,14 +55,15 @@ class NodeTraceWriter : public AsyncTraceWriter { // Used to wait until async handles have been closed. ConditionVariable exit_cond_; int fd_ = -1; - std::queue write_req_queue_; + uv_fs_t write_req_; + std::queue write_req_queue_; int num_write_requests_ = 0; int highest_request_id_completed_ = 0; int total_traces_ = 0; int file_num_ = 0; const std::string& log_file_pattern_; std::ostringstream stream_; - TraceWriter* json_trace_writer_ = nullptr; + std::unique_ptr json_trace_writer_; bool exited_ = false; }; diff --git a/src/util.cc b/src/util.cc index 3e808e13fe87d8..66be18eae2d150 100644 --- a/src/util.cc +++ b/src/util.cc @@ -24,6 +24,7 @@ #include "node_internals.h" #include "uv.h" #include +#include namespace node { @@ -118,4 +119,17 @@ void GetHumanReadableProcessName(char (*name)[1024]) { snprintf(*name, sizeof(*name), "%s[%d]", title, uv_os_getpid()); } +std::set ParseCommaSeparatedSet(const std::string& in) { + std::set out; + if (in.empty()) + return out; + std::istringstream in_stream(in); + while (in_stream.good()) { + std::string item; + getline(in_stream, item, ','); + out.emplace(std::move(item)); + } + return out; +} + } // namespace node diff --git a/src/util.h b/src/util.h index f7dcc5ea35abf8..a9fce79ebeaec1 100644 --- a/src/util.h +++ b/src/util.h @@ -36,6 +36,7 @@ #include #include // std::function +#include namespace node { @@ -476,6 +477,8 @@ struct FunctionDeleter { template using DeleteFnPtr = typename FunctionDeleter::Pointer; +std::set ParseCommaSeparatedSet(const std::string& in); + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/parallel/test-tracing-no-crash.js b/test/parallel/test-tracing-no-crash.js new file mode 100644 index 00000000000000..816bd95df02e29 --- /dev/null +++ b/test/parallel/test-tracing-no-crash.js @@ -0,0 +1,14 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { spawn } = require('child_process'); + +function CheckNoSignalAndErrorCodeOne(code, signal) { + assert.strictEqual(null, signal); + assert.strictEqual(1, code); +} + +const child = spawn(process.execPath, + ['--trace-event-categories', 'madeup', '-e', + 'throw new Error()'], { stdio: 'inherit' }); +child.on('exit', common.mustCall(CheckNoSignalAndErrorCodeOne));