Skip to content

Commit

Permalink
src: make EndStartedProfilers an exit hook
Browse files Browse the repository at this point in the history
Run `EndStartedProfilers` on Environment teardown.

This is part of a series of changes to make embedding easier, by
requiring fewer internal methods to build a fully functioning
Node.js instance.

PR-URL: #30229
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
  • Loading branch information
addaleax committed Nov 6, 2019
1 parent 55f98df commit 5bf4372
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 18 deletions.
27 changes: 21 additions & 6 deletions src/inspector_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "diagnosticfilename-inl.h"
#include "memory_tracker-inl.h"
#include "node_file.h"
#include "node_errors.h"
#include "node_internals.h"
#include "util-inl.h"
#include "v8-inspector.h"
Expand All @@ -13,6 +14,7 @@
namespace node {
namespace profiler {

using errors::TryCatchScope;
using v8::Context;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -219,12 +221,21 @@ void V8CoverageConnection::WriteProfile(Local<String> message) {
}

// append source-map cache information to coverage object:
Local<Function> source_map_cache_getter = env_->source_map_cache_getter();
Local<Value> source_map_cache_v;
if (!source_map_cache_getter->Call(env()->context(),
Undefined(isolate), 0, nullptr)
.ToLocal(&source_map_cache_v)) {
return;
{
TryCatchScope try_catch(env());
{
Isolate::AllowJavascriptExecutionScope allow_js_here(isolate);
Local<Function> source_map_cache_getter = env_->source_map_cache_getter();
if (!source_map_cache_getter->Call(
context, Undefined(isolate), 0, nullptr)
.ToLocal(&source_map_cache_v)) {
return;
}
}
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
PrintCaughtException(isolate, context, try_catch);
}
}
// Avoid writing to disk if no source-map data:
if (!source_map_cache_v->IsUndefined()) {
Expand Down Expand Up @@ -351,7 +362,7 @@ void V8HeapProfilerConnection::End() {

// For now, we only support coverage profiling, but we may add more
// in the future.
void EndStartedProfilers(Environment* env) {
static void EndStartedProfilers(Environment* env) {
Debug(env, DebugCategory::INSPECTOR_PROFILER, "EndStartedProfilers\n");
V8ProfilerConnection* connection = env->cpu_profiler_connection();
if (connection != nullptr && !connection->ending()) {
Expand Down Expand Up @@ -390,6 +401,10 @@ std::string GetCwd(Environment* env) {
}

void StartProfilers(Environment* env) {
AtExit(env, [](void* env) {
EndStartedProfilers(static_cast<Environment*>(env));
}, env);

Isolate* isolate = env->isolate();
Local<String> coverage_str = env->env_vars()->Get(
isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"))
Expand Down
1 change: 0 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ static const unsigned kMaxSignal = 32;

void WaitForInspectorDisconnect(Environment* env) {
#if HAVE_INSPECTOR
profiler::EndStartedProfilers(env);

if (env->inspector_agent()->IsActive()) {
// Restore signal dispositions, the app is done and is no longer
Expand Down
4 changes: 1 addition & 3 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -993,9 +993,7 @@ void TriggerUncaughtException(Isolate* isolate,

// Now we are certain that the exception is fatal.
ReportFatalException(env, error, message, EnhanceFatalException::kEnhance);
#if HAVE_INSPECTOR
profiler::EndStartedProfilers(env);
#endif
RunAtExit(env);

// If the global uncaught exception handler sets process.exitCode,
// exit with that code. Otherwise, exit with 1.
Expand Down
1 change: 0 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
#if HAVE_INSPECTOR
namespace profiler {
void StartProfilers(Environment* env);
void EndStartedProfilers(Environment* env);
}
#endif // HAVE_INSPECTOR

Expand Down
13 changes: 9 additions & 4 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,15 @@ static void Kill(const FunctionCallbackInfo<Value>& args) {
if (!args[0]->Int32Value(context).To(&pid)) return;
int sig;
if (!args[1]->Int32Value(context).To(&sig)) return;
// TODO(joyeecheung): white list the signals?

#if HAVE_INSPECTOR
profiler::EndStartedProfilers(env);
#endif
uv_pid_t own_pid = uv_os_getpid();
if (sig > 0 &&
(pid == 0 || pid == -1 || pid == own_pid || pid == -own_pid) &&
!HasSignalJSHandler(sig)) {
// This is most likely going to terminate this process.
// It's not an exact method but it might be close enough.
RunAtExit(env);
}

int err = uv_kill(pid, sig);
args.GetReturnValue().Set(err);
Expand Down Expand Up @@ -428,6 +432,7 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {

static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
RunAtExit(env);
WaitForInspectorDisconnect(env);
int code = args[0]->Int32Value(env->context()).FromMaybe(0);
env->Exit(code);
Expand Down
3 changes: 0 additions & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,6 @@ void Worker::Run() {
if (exit_code_ == 0 && !stopped)
exit_code_ = exit_code;

#if HAVE_INSPECTOR
profiler::EndStartedProfilers(env_.get());
#endif
Debug(this, "Exiting thread for worker %llu with exit code %d",
thread_id_, exit_code_);
}
Expand Down

0 comments on commit 5bf4372

Please sign in to comment.