Skip to content

Commit 619b718

Browse files
addaleaxtargos
authored andcommittedDec 1, 2019
src: make EndStartedProfilers an exit hook
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 <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
1 parent 5ec3573 commit 619b718

6 files changed

+31
-18
lines changed
 

‎src/inspector_profiler.cc

+21-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "diagnosticfilename-inl.h"
55
#include "memory_tracker-inl.h"
66
#include "node_file.h"
7+
#include "node_errors.h"
78
#include "node_internals.h"
89
#include "util-inl.h"
910
#include "v8-inspector.h"
@@ -13,6 +14,7 @@
1314
namespace node {
1415
namespace profiler {
1516

17+
using errors::TryCatchScope;
1618
using v8::Context;
1719
using v8::Function;
1820
using v8::FunctionCallbackInfo;
@@ -219,12 +221,21 @@ void V8CoverageConnection::WriteProfile(Local<String> message) {
219221
}
220222

221223
// append source-map cache information to coverage object:
222-
Local<Function> source_map_cache_getter = env_->source_map_cache_getter();
223224
Local<Value> source_map_cache_v;
224-
if (!source_map_cache_getter->Call(env()->context(),
225-
Undefined(isolate), 0, nullptr)
226-
.ToLocal(&source_map_cache_v)) {
227-
return;
225+
{
226+
TryCatchScope try_catch(env());
227+
{
228+
Isolate::AllowJavascriptExecutionScope allow_js_here(isolate);
229+
Local<Function> source_map_cache_getter = env_->source_map_cache_getter();
230+
if (!source_map_cache_getter->Call(
231+
context, Undefined(isolate), 0, nullptr)
232+
.ToLocal(&source_map_cache_v)) {
233+
return;
234+
}
235+
}
236+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
237+
PrintCaughtException(isolate, context, try_catch);
238+
}
228239
}
229240
// Avoid writing to disk if no source-map data:
230241
if (!source_map_cache_v->IsUndefined()) {
@@ -351,7 +362,7 @@ void V8HeapProfilerConnection::End() {
351362

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

392403
void StartProfilers(Environment* env) {
404+
AtExit(env, [](void* env) {
405+
EndStartedProfilers(static_cast<Environment*>(env));
406+
}, env);
407+
393408
Isolate* isolate = env->isolate();
394409
Local<String> coverage_str = env->env_vars()->Get(
395410
isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"))

‎src/node.cc

-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ static const unsigned kMaxSignal = 32;
157157

158158
void WaitForInspectorDisconnect(Environment* env) {
159159
#if HAVE_INSPECTOR
160-
profiler::EndStartedProfilers(env);
161160

162161
if (env->inspector_agent()->IsActive()) {
163162
// Restore signal dispositions, the app is done and is no longer

‎src/node_errors.cc

+1-3
Original file line numberDiff line numberDiff line change
@@ -980,9 +980,7 @@ void TriggerUncaughtException(Isolate* isolate,
980980

981981
// Now we are certain that the exception is fatal.
982982
ReportFatalException(env, error, message, EnhanceFatalException::kEnhance);
983-
#if HAVE_INSPECTOR
984-
profiler::EndStartedProfilers(env);
985-
#endif
983+
RunAtExit(env);
986984

987985
// If the global uncaught exception handler sets process.exitCode,
988986
// exit with that code. Otherwise, exit with 1.

‎src/node_internals.h

-1
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,6 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
310310
#if HAVE_INSPECTOR
311311
namespace profiler {
312312
void StartProfilers(Environment* env);
313-
void EndStartedProfilers(Environment* env);
314313
}
315314
#endif // HAVE_INSPECTOR
316315

‎src/node_process_methods.cc

+9-4
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,15 @@ static void Kill(const FunctionCallbackInfo<Value>& args) {
165165
if (!args[0]->Int32Value(context).To(&pid)) return;
166166
int sig;
167167
if (!args[1]->Int32Value(context).To(&sig)) return;
168-
// TODO(joyeecheung): white list the signals?
169168

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

174178
int err = uv_kill(pid, sig);
175179
args.GetReturnValue().Set(err);
@@ -421,6 +425,7 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {
421425

422426
static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
423427
Environment* env = Environment::GetCurrent(args);
428+
RunAtExit(env);
424429
WaitForInspectorDisconnect(env);
425430
int code = args[0]->Int32Value(env->context()).FromMaybe(0);
426431
env->Exit(code);

‎src/node_worker.cc

-3
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,6 @@ void Worker::Run() {
323323
if (exit_code_ == 0 && !stopped)
324324
exit_code_ = exit_code;
325325

326-
#if HAVE_INSPECTOR
327-
profiler::EndStartedProfilers(env_.get());
328-
#endif
329326
Debug(this, "Exiting thread for worker %llu with exit code %d",
330327
thread_id_, exit_code_);
331328
}

0 commit comments

Comments
 (0)