From c5cc04f43e9a922e0b6f150ee901b1b713f555c1 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Mon, 8 Jan 2018 21:05:14 -0500 Subject: [PATCH] perf_hooks: fix scheduling regression 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. --- src/node_perf.cc | 24 +++++++++++++++++------- test/parallel/test-performance-gc.js | 10 ++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index 6a59d50fb64ff3..97d3a2d99522fe 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -182,8 +182,9 @@ void SetupPerformanceObservers(const FunctionCallbackInfo& args) { } // Creates a GC Performance Entry and passes it to observers -void PerformanceGCCallback(Environment* env, void* ptr) { - GCPerformanceEntry* entry = static_cast(ptr); +void PerformanceGCCallback(uv_async_t* handle) { + GCPerformanceEntry* entry = static_cast(handle->data); + Environment* env = entry->env(); HandleScope scope(env->isolate()); Local context = env->context(); @@ -200,6 +201,10 @@ void PerformanceGCCallback(Environment* env, void* ptr) { } delete entry; + auto closeCB = [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }; + uv_close(reinterpret_cast(handle), closeCB); } // Marks the start of a GC cycle @@ -216,11 +221,16 @@ void MarkGarbageCollectionEnd(Isolate* isolate, v8::GCCallbackFlags flags, void* data) { Environment* env = static_cast(data); - env->SetImmediate(PerformanceGCCallback, - new GCPerformanceEntry(env, - static_cast(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(async)); + async->data = + new GCPerformanceEntry(env, + static_cast(type), + performance_last_gc_start_mark_, + PERFORMANCE_NOW()); + CHECK_EQ(0, uv_async_send(async)); } diff --git a/test/parallel/test-performance-gc.js b/test/parallel/test-performance-gc.js index 89a9041c1c1159..ec0714ea50034d 100644 --- a/test/parallel/test-performance-gc.js +++ b/test/parallel/test-performance-gc.js @@ -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(); + }); +}