Skip to content

Commit

Permalink
perf_hooks: fix scheduling regression
Browse files Browse the repository at this point in the history
Scheduling a PerformanceGCCallback should not keep the
loop alive but due to the recent switch to using the
native SetImmediate method, it does. Go back to using
uv_async_t and add a regression test.

Backport-PR-URL: #18050
PR-URL: #18051
Fixes: #18047
Refs: #18020
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
apapirovski authored and MylesBorins committed Jan 9, 2018
1 parent ab17d84 commit 89233a7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
24 changes: 17 additions & 7 deletions src/node_perf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,9 @@ void SetupPerformanceObservers(const FunctionCallbackInfo<Value>& args) {
}

// Creates a GC Performance Entry and passes it to observers
void PerformanceGCCallback(Environment* env, void* ptr) {
GCPerformanceEntry* entry = static_cast<GCPerformanceEntry*>(ptr);
void PerformanceGCCallback(uv_async_t* handle) {
GCPerformanceEntry* entry = static_cast<GCPerformanceEntry*>(handle->data);
Environment* env = entry->env();
HandleScope scope(env->isolate());
Local<Context> context = env->context();

Expand All @@ -200,6 +201,10 @@ void PerformanceGCCallback(Environment* env, void* ptr) {
}

delete entry;
auto closeCB = [](uv_handle_t* handle) {
delete reinterpret_cast<uv_async_t*>(handle);
};
uv_close(reinterpret_cast<uv_handle_t*>(handle), closeCB);
}

// Marks the start of a GC cycle
Expand All @@ -216,11 +221,16 @@ void MarkGarbageCollectionEnd(Isolate* isolate,
v8::GCCallbackFlags flags,
void* data) {
Environment* env = static_cast<Environment*>(data);
env->SetImmediate(PerformanceGCCallback,
new GCPerformanceEntry(env,
static_cast<PerformanceGCKind>(type),
performance_last_gc_start_mark_,
PERFORMANCE_NOW()));
uv_async_t* async = new uv_async_t();
if (uv_async_init(env->event_loop(), async, PerformanceGCCallback))
return delete async;
uv_unref(reinterpret_cast<uv_handle_t*>(async));
async->data =
new GCPerformanceEntry(env,
static_cast<PerformanceGCKind>(type),
performance_last_gc_start_mark_,
PERFORMANCE_NOW());
CHECK_EQ(0, uv_async_send(async));
}


Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-performance-gc.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,13 @@ const kinds = [
// Keep the event loop alive to witness the GC async callback happen.
setImmediate(() => setImmediate(() => 0));
}

// GC should not keep the event loop alive
{
let didCall = false;
process.on('beforeExit', () => {
assert(!didCall);
didCall = true;
global.gc();
});
}

0 comments on commit 89233a7

Please sign in to comment.