Skip to content

Commit

Permalink
src: fix DTrace GC callbacks DCHECKs and add cleanup
Browse files Browse the repository at this point in the history
Use the variant of GC callbacks that takes data to
avoid running into DCHECKs when multiple Environments try to add
the same callback to the same isolate multiple times.
In addition, remove the callbacks in the Environment cleanup hook.

PR-URL: nodejs#26742
Fixes: nodejs#26736
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and danbev committed Mar 21, 2019
1 parent 67c9f42 commit f47adfb
Showing 1 changed file with 18 additions and 6 deletions.
24 changes: 18 additions & 6 deletions src/node_dtrace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,19 @@ void DTRACE_HTTP_CLIENT_RESPONSE(const FunctionCallbackInfo<Value>& args) {
NODE_HTTP_CLIENT_RESPONSE(&conn, conn.remote, conn.port, conn.fd);
}


void dtrace_gc_start(Isolate* isolate, GCType type, GCCallbackFlags flags) {
void dtrace_gc_start(Isolate* isolate,
GCType type,
GCCallbackFlags flags,
void* data) {
// Previous versions of this probe point only logged type and flags.
// That's why for reasons of backwards compatibility the isolate goes last.
NODE_GC_START(type, flags, isolate);
}


void dtrace_gc_done(Isolate* isolate, GCType type, GCCallbackFlags flags) {
void dtrace_gc_done(Isolate* isolate,
GCType type,
GCCallbackFlags flags,
void* data) {
// Previous versions of this probe point only logged type and flags.
// That's why for reasons of backwards compatibility the isolate goes last.
NODE_GC_DONE(type, flags, isolate);
Expand All @@ -272,8 +276,16 @@ void InitDTrace(Environment* env) {
}
#endif

env->isolate()->AddGCPrologueCallback(dtrace_gc_start);
env->isolate()->AddGCEpilogueCallback(dtrace_gc_done);
// We need to use the variant of GC callbacks that takes data to
// avoid running into DCHECKs when multiple Environments try to add
// the same callback to the same isolate multiple times.
env->isolate()->AddGCPrologueCallback(dtrace_gc_start, env);
env->isolate()->AddGCEpilogueCallback(dtrace_gc_done, env);
env->AddCleanupHook([](void* data) {
Environment* env = static_cast<Environment*>(data);
env->isolate()->RemoveGCPrologueCallback(dtrace_gc_start, env);
env->isolate()->RemoveGCEpilogueCallback(dtrace_gc_done, env);
}, env);
}

void InitializeDTrace(Local<Object> target,
Expand Down

0 comments on commit f47adfb

Please sign in to comment.