From 68d33c692e96d0c6b21e96f73a4dbdedc0b5431e Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
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.

PR-URL: https://github.com/nodejs/node/pull/18051
Fixes: https://github.com/nodejs/node/issues/18047
Refs: https://github.com/nodejs/node/pull/18020
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 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 d37f8ad002ec22..97f351a9748c8b 100644
--- a/src/node_perf.cc
+++ b/src/node_perf.cc
@@ -184,8 +184,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();
 
@@ -203,6 +204,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
@@ -219,11 +224,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));
 }
 
 
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();
+  });
+}