From f1ac1599a39646bec687e63856329e6d78cd94b5 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 29 Apr 2019 18:50:51 +0800 Subject: [PATCH 1/3] src: refactor profile initialization - Process and store --cpu-prof-dir and --cpu-prof-name during Environment creation - Start profilers in on `profiler::StartProfilers()` --- src/env-inl.h | 12 +++--- src/env.cc | 14 ------- src/env.h | 10 ++--- src/inspector_profiler.cc | 82 ++++++++++++++++++++++++--------------- src/node.cc | 15 +------ src/node_internals.h | 3 +- 6 files changed, 62 insertions(+), 74 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 36f8506bafb5a7..648bde590b4ede 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -673,16 +673,16 @@ Environment::cpu_profiler_connection() { return cpu_profiler_connection_.get(); } -inline void Environment::set_cpu_profile_path(const std::string& path) { - cpu_profile_path_ = path; +inline void Environment::set_cpu_prof_name(const std::string& name) { + cpu_prof_name_ = name; } -inline const std::string& Environment::cpu_profile_path() const { - return cpu_profile_path_; +inline const std::string& Environment::cpu_prof_name() const { + return cpu_prof_name_; } -inline void Environment::set_cpu_prof_dir(const std::string& path) { - cpu_prof_dir_ = path; +inline void Environment::set_cpu_prof_dir(const std::string& dir) { + cpu_prof_dir_ = dir; } inline const std::string& Environment::cpu_prof_dir() const { diff --git a/src/env.cc b/src/env.cc index ad43b13607006e..197cd5bd904208 100644 --- a/src/env.cc +++ b/src/env.cc @@ -907,20 +907,6 @@ void Environment::stop_sub_worker_contexts() { #if HAVE_INSPECTOR -void Environment::InitializeCPUProfDir(const std::string& dir) { - if (!dir.empty()) { - cpu_prof_dir_ = dir; - return; - } - char cwd[CWD_BUFSIZE]; - size_t size = CWD_BUFSIZE; - int err = uv_cwd(cwd, &size); - // TODO(joyeecheung): fallback to exec path / argv[0] - CHECK_EQ(err, 0); - CHECK_GT(size, 0); - cpu_prof_dir_ = cwd; -} - #endif // HAVE_INSPECTOR void MemoryTracker::TrackField(const char* edge_name, diff --git a/src/env.h b/src/env.h index d4991535186d51..f607c784c10606 100644 --- a/src/env.h +++ b/src/env.h @@ -1139,13 +1139,11 @@ class Environment : public MemoryRetainer { std::unique_ptr connection); profiler::V8CpuProfilerConnection* cpu_profiler_connection(); - inline void set_cpu_profile_path(const std::string& path); - inline const std::string& cpu_profile_path() const; + inline void set_cpu_prof_name(const std::string& name); + inline const std::string& cpu_prof_name() const; - inline void set_cpu_prof_dir(const std::string& path); + inline void set_cpu_prof_dir(const std::string& dir); inline const std::string& cpu_prof_dir() const; - - void InitializeCPUProfDir(const std::string& dir); #endif // HAVE_INSPECTOR private: @@ -1183,7 +1181,7 @@ class Environment : public MemoryRetainer { std::unique_ptr cpu_profiler_connection_; std::string coverage_directory_; std::string cpu_prof_dir_; - std::string cpu_profile_path_; + std::string cpu_prof_name_; #endif // HAVE_INSPECTOR std::shared_ptr options_; diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index 3507ae4ef2f1e0..a50ca0f16419dc 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -209,26 +209,26 @@ void V8CpuProfilerConnection::OnMessage( } void V8CpuProfilerConnection::WriteCpuProfile(Local message) { - const std::string& path = env()->cpu_profile_path(); - CHECK(!path.empty()); - std::string directory = path.substr(0, path.find_last_of(kPathSeparator)); - if (directory != path) { - uv_fs_t req; - int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr); - uv_fs_req_cleanup(&req); - if (ret < 0 && ret != UV_EEXIST) { - char err_buf[128]; - uv_err_name_r(ret, err_buf, sizeof(err_buf)); - fprintf(stderr, - "%s: Failed to create cpu profile directory %s\n", - err_buf, - directory.c_str()); - return; - } + const std::string& filename = env()->cpu_prof_name(); + const std::string& directory = env()->cpu_prof_dir(); + CHECK(!filename.empty()); + CHECK(!directory.empty()); + uv_fs_t req; + int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr); + uv_fs_req_cleanup(&req); + if (ret < 0 && ret != UV_EEXIST) { + char err_buf[128]; + uv_err_name_r(ret, err_buf, sizeof(err_buf)); + fprintf(stderr, + "%s: Failed to create cpu profile directory %s\n", + err_buf, + directory.c_str()); + return; } MaybeLocal result = GetResult(message); + std::string target = directory + kPathSeparator + filename; if (!result.IsEmpty()) { - WriteResult(path.c_str(), result.ToLocalChecked()); + WriteResult(target.c_str(), result.ToLocalChecked()); } } @@ -312,24 +312,42 @@ void EndStartedProfilers(Environment* env) { } } -void StartCoverageCollection(Environment* env) { - CHECK_NULL(env->coverage_connection()); - env->set_coverage_connection(std::make_unique(env)); - env->coverage_connection()->Start(); +std::string GetCwd() { + char cwd[CWD_BUFSIZE]; + size_t size = CWD_BUFSIZE; + int err = uv_cwd(cwd, &size); + // This can fail if the cwd is deleted. + // TODO(joyeecheung): store this in the Environment during Environment + // creation and fallback to exec_path and argv0, then we no longer need + // SetCoverageDirectory(). + CHECK_EQ(err, 0); + CHECK_GT(size, 0); + return cwd; } -void StartCpuProfiling(Environment* env, const std::string& profile_name) { - std::string path = env->cpu_prof_dir() + std::string(kPathSeparator); - if (profile_name.empty()) { - DiagnosticFilename filename(env, "CPU", "cpuprofile"); - path += *filename; - } else { - path += profile_name; +void StartProfilers(Environment* env) { + Isolate* isolate = env->isolate(); + Local coverage_str = env->env_vars()->Get( + isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE")); + if (!coverage_str.IsEmpty() && coverage_str->Length() > 0) { + CHECK_NULL(env->coverage_connection()); + env->set_coverage_connection(std::make_unique(env)); + env->coverage_connection()->Start(); + } + if (env->options()->cpu_prof) { + const std::string& dir = env->options()->cpu_prof_dir; + env->set_cpu_prof_dir(dir.empty() ? GetCwd() : dir); + if (env->options()->cpu_prof_name.empty()) { + DiagnosticFilename filename(env, "CPU", "cpuprofile"); + env->set_cpu_prof_name(*filename); + } else { + env->set_cpu_prof_name(env->options()->cpu_prof_name); + } + CHECK_NULL(env->cpu_profiler_connection()); + env->set_cpu_profiler_connection( + std::make_unique(env)); + env->cpu_profiler_connection()->Start(); } - env->set_cpu_profile_path(std::move(path)); - env->set_cpu_profiler_connection( - std::make_unique(env)); - env->cpu_profiler_connection()->Start(); } static void SetCoverageDirectory(const FunctionCallbackInfo& args) { diff --git a/src/node.cc b/src/node.cc index 636a92eab3f760..d611460f4f2921 100644 --- a/src/node.cc +++ b/src/node.cc @@ -227,21 +227,8 @@ MaybeLocal RunBootstrapping(Environment* env) { Isolate* isolate = env->isolate(); Local context = env->context(); - Local coverage_str = env->env_vars()->Get( - isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE")); - if (!coverage_str.IsEmpty() && coverage_str->Length() > 0) { #if HAVE_INSPECTOR - profiler::StartCoverageCollection(env); -#else - fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector\n"); -#endif // HAVE_INSPECTOR - } - -#if HAVE_INSPECTOR - if (env->options()->cpu_prof) { - env->InitializeCPUProfDir(env->options()->cpu_prof_dir); - profiler::StartCpuProfiling(env, env->options()->cpu_prof_name); - } + profiler::StartProfilers(env); #endif // HAVE_INSPECTOR // Add a reference to the global object diff --git a/src/node_internals.h b/src/node_internals.h index fc924e3e1637ce..f89fbe291cd253 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -325,8 +325,7 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params); #if HAVE_INSPECTOR namespace profiler { -void StartCoverageCollection(Environment* env); -void StartCpuProfiling(Environment* env, const std::string& profile_name); +void StartProfilers(Environment* env); void EndStartedProfilers(Environment* env); } #endif // HAVE_INSPECTOR From 026d821aee31befec9104630f89ae6627ca414e7 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 29 Apr 2019 19:45:33 +0800 Subject: [PATCH 2/3] src: refactor V8ProfilerConnection to be more reusable Now the subclasses only need to override methods to - Get the profile node from the profiler response message object - Return the directory and file path And the V8ProfilerConnection() would handle the rest of profile serialization and message parsing. This makes it easier to add other types of profiles in the future. --- src/inspector_profiler.cc | 233 ++++++++++++++++++-------------------- src/inspector_profiler.h | 49 +++++--- 2 files changed, 140 insertions(+), 142 deletions(-) diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index a50ca0f16419dc..50269885cd3d4e 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -50,45 +50,53 @@ void V8ProfilerConnection::DispatchMessage(Local message) { session_->Dispatch(ToProtocolString(env()->isolate(), message)->string()); } -bool V8ProfilerConnection::WriteResult(const char* path, Local result) { - int ret = WriteFileSync(env()->isolate(), path, result); +static void WriteResult(Environment* env, + const char* path, + Local result) { + int ret = WriteFileSync(env->isolate(), path, result); if (ret != 0) { char err_buf[128]; uv_err_name_r(ret, err_buf, sizeof(err_buf)); fprintf(stderr, "%s: Failed to write file %s\n", err_buf, path); - return false; + return; } - Debug( - env(), DebugCategory::INSPECTOR_PROFILER, "Written result to %s\n", path); - return true; + Debug(env, DebugCategory::INSPECTOR_PROFILER, "Written result to %s\n", path); } -void V8CoverageConnection::OnMessage(const v8_inspector::StringView& message) { - Debug(env(), +void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend( + const v8_inspector::StringView& message) { + Environment* env = connection_->env(); + Isolate* isolate = env->isolate(); + HandleScope handle_scope(isolate); + Context::Scope context_scope(env->context()); + + const char* type = connection_->type(); + Debug(env, DebugCategory::INSPECTOR_PROFILER, - "Receive coverage message, ending = %s\n", - ending_ ? "true" : "false"); - if (!ending_) { + "Receive %s profile message, ending = %s\n", + type, + connection_->ending() ? "true" : "false"); + if (!connection_->ending()) { return; } - Isolate* isolate = env()->isolate(); - Local context = env()->context(); - HandleScope handle_scope(isolate); - Context::Scope context_scope(context); - Local result; + + // Convert StringView to a Local + Local message_str; if (!String::NewFromTwoByte(isolate, message.characters16(), NewStringType::kNormal, message.length()) - .ToLocal(&result)) { - fprintf(stderr, "Failed to covert coverage message\n"); + .ToLocal(&message_str)) { + fprintf(stderr, "Failed to covert %s profile message\n", type); + return; } - WriteCoverage(result); + + connection_->WriteProfile(message_str); } -bool V8CoverageConnection::WriteCoverage(Local message) { - const std::string& directory = env()->coverage_directory(); - CHECK(!directory.empty()); +static bool EnsureDirectory(const std::string& directory, const char* type) { + // TODO(joyeecheung): MKDirp functions should stat first to see if + // the directory exists. uv_fs_t req; int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr); uv_fs_req_cleanup(&req); @@ -96,12 +104,16 @@ bool V8CoverageConnection::WriteCoverage(Local message) { char err_buf[128]; uv_err_name_r(ret, err_buf, sizeof(err_buf)); fprintf(stderr, - "%s: Failed to create coverage directory %s\n", + "%s: Failed to create %s profile directory %s\n", err_buf, + type, directory.c_str()); return false; } + return true; +} +std::string V8CoverageConnection::GetFilename() const { std::string thread_id = std::to_string(env()->thread_id()); std::string pid = std::to_string(uv_os_getpid()); std::string timestamp = std::to_string( @@ -113,44 +125,79 @@ bool V8CoverageConnection::WriteCoverage(Local message) { pid.c_str(), timestamp.c_str(), thread_id.c_str()); - std::string target = directory + kPathSeparator + filename; - MaybeLocal result = GetResult(message); - if (result.IsEmpty()) { - return false; - } - return WriteResult(target.c_str(), result.ToLocalChecked()); + return filename; } -MaybeLocal V8CoverageConnection::GetResult(Local message) { - Local context = env()->context(); - Isolate* isolate = env()->isolate(); +static MaybeLocal ParseProfile(Environment* env, + Local message, + const char* type) { + Local context = env->context(); + Isolate* isolate = env->isolate(); + + // Get message.result from the response Local parsed; if (!v8::JSON::Parse(context, message).ToLocal(&parsed) || !parsed->IsObject()) { - fprintf(stderr, "Failed to parse coverage result as JSON object\n"); - return MaybeLocal(); + fprintf(stderr, "Failed to parse %s profile result as JSON object\n", type); + return MaybeLocal(); } Local result_v; if (!parsed.As() ->Get(context, FIXED_ONE_BYTE_STRING(isolate, "result")) .ToLocal(&result_v)) { - fprintf(stderr, "Failed to get result from coverage message\n"); - return MaybeLocal(); + fprintf(stderr, "Failed to get 'result' from %s profile message\n", type); + return MaybeLocal(); } - if (result_v->IsUndefined()) { - fprintf(stderr, "'result' from coverage message is undefined\n"); - return MaybeLocal(); + if (!result_v->IsObject()) { + fprintf( + stderr, "'result' from %s profile message is not an object\n", type); + return MaybeLocal(); } + return result_v.As(); +} + +void V8ProfilerConnection::WriteProfile(Local message) { + Local context = env_->context(); + + // Get message.result from the response + Local result; + if (!ParseProfile(env_, message, type()).ToLocal(&result)) { + return; + } + // Generate the profile output from the subclass. + Local profile; + if (!GetProfile(result).ToLocal(&profile)) { + return; + } Local result_s; - if (!v8::JSON::Stringify(context, result_v).ToLocal(&result_s)) { - fprintf(stderr, "Failed to stringify coverage result\n"); - return MaybeLocal(); + if (!v8::JSON::Stringify(context, profile).ToLocal(&result_s)) { + fprintf(stderr, "Failed to stringify %s profile result\n", type()); + return; } - return result_s; + // Create the directory if necessary + std::string directory = GetDirectory(); + DCHECK(!directory.empty()); + if (!EnsureDirectory(directory, type())) { + return; + } + + std::string filename = GetFilename(); + DCHECK(!filename.empty()); + std::string path = directory + kPathSeparator + filename; + + WriteResult(env_, path.c_str(), result_s); +} + +MaybeLocal V8CoverageConnection::GetProfile(Local result) { + return result; +} + +std::string V8CoverageConnection::GetDirectory() const { + return env()->coverage_directory(); } void V8CoverageConnection::Start() { @@ -184,92 +231,28 @@ void V8CoverageConnection::End() { DispatchMessage(end); } -void V8CpuProfilerConnection::OnMessage( - const v8_inspector::StringView& message) { - Debug(env(), - DebugCategory::INSPECTOR_PROFILER, - "Receive cpu profiling message, ending = %s\n", - ending_ ? "true" : "false"); - if (!ending_) { - return; - } - Isolate* isolate = env()->isolate(); - HandleScope handle_scope(isolate); - Local context = env()->context(); - Context::Scope context_scope(context); - Local result; - if (!String::NewFromTwoByte(isolate, - message.characters16(), - NewStringType::kNormal, - message.length()) - .ToLocal(&result)) { - fprintf(stderr, "Failed to convert profiling message\n"); - } - WriteCpuProfile(result); +std::string V8CpuProfilerConnection::GetDirectory() const { + return env()->cpu_prof_dir(); } -void V8CpuProfilerConnection::WriteCpuProfile(Local message) { - const std::string& filename = env()->cpu_prof_name(); - const std::string& directory = env()->cpu_prof_dir(); - CHECK(!filename.empty()); - CHECK(!directory.empty()); - uv_fs_t req; - int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr); - uv_fs_req_cleanup(&req); - if (ret < 0 && ret != UV_EEXIST) { - char err_buf[128]; - uv_err_name_r(ret, err_buf, sizeof(err_buf)); - fprintf(stderr, - "%s: Failed to create cpu profile directory %s\n", - err_buf, - directory.c_str()); - return; - } - MaybeLocal result = GetResult(message); - std::string target = directory + kPathSeparator + filename; - if (!result.IsEmpty()) { - WriteResult(target.c_str(), result.ToLocalChecked()); - } +std::string V8CpuProfilerConnection::GetFilename() const { + return env()->cpu_prof_name(); } -MaybeLocal V8CpuProfilerConnection::GetResult(Local message) { - Local context = env()->context(); - Isolate* isolate = env()->isolate(); - Local parsed; - if (!v8::JSON::Parse(context, message).ToLocal(&parsed) || - !parsed->IsObject()) { - fprintf(stderr, "Failed to parse CPU profile result as JSON object\n"); - return MaybeLocal(); - } - - Local result_v; - if (!parsed.As() - ->Get(context, FIXED_ONE_BYTE_STRING(isolate, "result")) - .ToLocal(&result_v)) { - fprintf(stderr, "Failed to get result from CPU profile message\n"); - return MaybeLocal(); - } - - if (!result_v->IsObject()) { - fprintf(stderr, "'result' from CPU profile message is not an object\n"); - return MaybeLocal(); - } - +MaybeLocal V8CpuProfilerConnection::GetProfile(Local result) { Local profile_v; - if (!result_v.As() - ->Get(context, FIXED_ONE_BYTE_STRING(isolate, "profile")) + if (!result + ->Get(env()->context(), + FIXED_ONE_BYTE_STRING(env()->isolate(), "profile")) .ToLocal(&profile_v)) { fprintf(stderr, "'profile' from CPU profile result is undefined\n"); - return MaybeLocal(); + return MaybeLocal(); } - - Local result_s; - if (!v8::JSON::Stringify(context, profile_v).ToLocal(&result_s)) { - fprintf(stderr, "Failed to stringify CPU profile result\n"); - return MaybeLocal(); + if (!profile_v->IsObject()) { + fprintf(stderr, "'profile' from CPU profile result is not an Object\n"); + return MaybeLocal(); } - - return result_s; + return profile_v.As(); } void V8CpuProfilerConnection::Start() { @@ -298,16 +281,16 @@ void V8CpuProfilerConnection::End() { // in the future. void EndStartedProfilers(Environment* env) { Debug(env, DebugCategory::INSPECTOR_PROFILER, "EndStartedProfilers\n"); - V8ProfilerConnection* connection = env->coverage_connection(); + V8ProfilerConnection* connection = env->cpu_profiler_connection(); if (connection != nullptr && !connection->ending()) { - Debug( - env, DebugCategory::INSPECTOR_PROFILER, "Ending coverage collection\n"); + Debug(env, DebugCategory::INSPECTOR_PROFILER, "Ending cpu profiling\n"); connection->End(); } - connection = env->cpu_profiler_connection(); + connection = env->coverage_connection(); if (connection != nullptr && !connection->ending()) { - Debug(env, DebugCategory::INSPECTOR_PROFILER, "Ending cpu profiling\n"); + Debug( + env, DebugCategory::INSPECTOR_PROFILER, "Ending coverage collection\n"); connection->End(); } } diff --git a/src/inspector_profiler.h b/src/inspector_profiler.h index 7120819c13b070..cbe053e578cf8c 100644 --- a/src/inspector_profiler.h +++ b/src/inspector_profiler.h @@ -24,9 +24,7 @@ class V8ProfilerConnection { : connection_(connection) {} void SendMessageToFrontend( - const v8_inspector::StringView& message) override { - connection_->OnMessage(message); - } + const v8_inspector::StringView& message) override; private: V8ProfilerConnection* connection_; @@ -34,20 +32,30 @@ class V8ProfilerConnection { explicit V8ProfilerConnection(Environment* env); virtual ~V8ProfilerConnection() = default; - Environment* env() { return env_; } + + Environment* env() const { return env_; } + void DispatchMessage(v8::Local message); // Use DispatchMessage() to dispatch necessary inspector messages + // to start and end the profiling. virtual void Start() = 0; virtual void End() = 0; - // Override this to respond to the messages sent from the session. - virtual void OnMessage(const v8_inspector::StringView& message) = 0; - virtual bool ending() const = 0; - void DispatchMessage(v8::Local message); - // Write the result to a path - bool WriteResult(const char* path, v8::Local result); + // Return a descriptive name of the profile for debugging. + virtual const char* type() const = 0; + // Return if the profile is ending and the response can be parsed. + virtual bool ending() const = 0; + // Return the directory where the profile should be placed. + virtual std::string GetDirectory() const = 0; + // Return the filename the profile should be written as. + virtual std::string GetFilename() const = 0; + // Return the profile object parsed from `message.result`, + // which will be then written as a JSON. + virtual v8::MaybeLocal GetProfile( + v8::Local result) = 0; private: + void WriteProfile(v8::Local message); std::unique_ptr session_; Environment* env_ = nullptr; }; @@ -58,14 +66,18 @@ class V8CoverageConnection : public V8ProfilerConnection { void Start() override; void End() override; - void OnMessage(const v8_inspector::StringView& message) override; + + const char* type() const override { return type_.c_str(); } bool ending() const override { return ending_; } + std::string GetDirectory() const override; + std::string GetFilename() const override; + v8::MaybeLocal GetProfile(v8::Local result) override; + private: - bool WriteCoverage(v8::Local message); - v8::MaybeLocal GetResult(v8::Local message); std::unique_ptr session_; bool ending_ = false; + std::string type_ = "coverage"; }; class V8CpuProfilerConnection : public V8ProfilerConnection { @@ -75,15 +87,18 @@ class V8CpuProfilerConnection : public V8ProfilerConnection { void Start() override; void End() override; - void OnMessage(const v8_inspector::StringView& message) override; + + const char* type() const override { return type_.c_str(); } bool ending() const override { return ending_; } - private: - void WriteCpuProfile(v8::Local message); - v8::MaybeLocal GetResult(v8::Local message); + std::string GetDirectory() const override; + std::string GetFilename() const override; + v8::MaybeLocal GetProfile(v8::Local result) override; + private: std::unique_ptr session_; bool ending_ = false; + std::string type_ = "CPU"; }; } // namespace profiler From 80f8e65151bd7a207ccc96b37176ffd02d91fdfc Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 1 May 2019 16:41:19 +0800 Subject: [PATCH 3/3] fixup! src: refactor V8ProfilerConnection to be more reusable --- src/inspector_profiler.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index 50269885cd3d4e..e1f68c33b62dc5 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -70,6 +70,9 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend( HandleScope handle_scope(isolate); Context::Scope context_scope(env->context()); + // TODO(joyeecheung): always parse the message so that we can use the id to + // identify ending messages as well as printing the message in the debug + // output when there is an error. const char* type = connection_->type(); Debug(env, DebugCategory::INSPECTOR_PROFILER, @@ -80,7 +83,7 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend( return; } - // Convert StringView to a Local + // Convert StringView to a Local. Local message_str; if (!String::NewFromTwoByte(isolate, message.characters16(), @@ -96,7 +99,8 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend( static bool EnsureDirectory(const std::string& directory, const char* type) { // TODO(joyeecheung): MKDirp functions should stat first to see if - // the directory exists. + // the directory exists. We should also use *at functions here and in + // MKDirp to avoid TOCTOU bugs at least on platforms with those APIs. uv_fs_t req; int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr); uv_fs_req_cleanup(&req); @@ -162,7 +166,7 @@ static MaybeLocal ParseProfile(Environment* env, void V8ProfilerConnection::WriteProfile(Local message) { Local context = env_->context(); - // Get message.result from the response + // Get message.result from the response. Local result; if (!ParseProfile(env_, message, type()).ToLocal(&result)) { return; @@ -178,7 +182,7 @@ void V8ProfilerConnection::WriteProfile(Local message) { return; } - // Create the directory if necessary + // Create the directory if necessary. std::string directory = GetDirectory(); DCHECK(!directory.empty()); if (!EnsureDirectory(directory, type())) {