From 5ddef2988ba7a1d8112e8f25d370e0e2164555cd Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Thu, 18 Jan 2018 16:52:51 -0500
Subject: [PATCH] async_wrap: schedule destroy hook as unref

Since the `DestroyAsyncIdsCallback` in Node.js can be scheduled as
a result of GC, that means that it can accidentally keep the event
loop open when it shouldn't.

Replace `SetImmediate` with the newly introduced `SetUnrefImmediate`
and in addition introduce RunBeforeExit callbacks, of which
`DestroyAsyncIdsCallback` is now the first. These callbacks will run
before the `beforeExit` event is emitted (which will now only be
emitted if the event loop is still not active).

PR-URL: https://github.com/nodejs/node/pull/18241
Fixes: https://github.com/nodejs/node/issues/18190
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
---
 src/async_wrap.cc | 10 +++++++++-
 src/env.cc        | 11 +++++++++++
 src/env.h         |  8 ++++++++
 src/node.cc       | 10 +++++++++-
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 5e9dc69f87839f..93bd3d4864fd5d 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -159,6 +159,12 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) {
   } while (!env->destroy_async_id_list()->empty());
 }
 
+static void DestroyAsyncIdsCallback(void* arg) {
+  Environment* env = static_cast<Environment*>(arg);
+  if (!env->destroy_async_id_list()->empty())
+    DestroyAsyncIdsCallback(env, nullptr);
+}
+
 
 void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) {
   AsyncHooks* async_hooks = env->async_hooks();
@@ -501,6 +507,8 @@ void AsyncWrap::Initialize(Local<Object> target,
   Isolate* isolate = env->isolate();
   HandleScope scope(isolate);
 
+  env->BeforeExit(DestroyAsyncIdsCallback, env);
+
   env->SetMethod(target, "setupHooks", SetupHooks);
   env->SetMethod(target, "pushAsyncIds", PushAsyncIds);
   env->SetMethod(target, "popAsyncIds", PopAsyncIds);
@@ -662,7 +670,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
     return;
 
   if (env->destroy_async_id_list()->empty()) {
-    env->SetImmediate(DestroyAsyncIdsCallback, nullptr);
+    env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr);
   }
 
   env->destroy_async_id_list()->push_back(async_id);
diff --git a/src/env.cc b/src/env.cc
index 17cdbbb79f9af1..8881f37b18e38d 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -209,6 +209,17 @@ void Environment::PrintSyncTrace() const {
   fflush(stderr);
 }
 
+void Environment::RunBeforeExitCallbacks() {
+  for (BeforeExitCallback before_exit : before_exit_functions_) {
+    before_exit.cb_(before_exit.arg_);
+  }
+  before_exit_functions_.clear();
+}
+
+void Environment::BeforeExit(void (*cb)(void* arg), void* arg) {
+  before_exit_functions_.push_back(BeforeExitCallback{cb, arg});
+}
+
 void Environment::RunAtExitCallbacks() {
   for (AtExitCallback at_exit : at_exit_functions_) {
     at_exit.cb_(at_exit.arg_);
diff --git a/src/env.h b/src/env.h
index d4577a67eac921..8540b439b67333 100644
--- a/src/env.h
+++ b/src/env.h
@@ -644,6 +644,8 @@ class Environment {
                                 const char* name,
                                 v8::FunctionCallback callback);
 
+  void BeforeExit(void (*cb)(void* arg), void* arg);
+  void RunBeforeExitCallbacks();
   void AtExit(void (*cb)(void* arg), void* arg);
   void RunAtExitCallbacks();
 
@@ -770,6 +772,12 @@ class Environment {
 
   double* fs_stats_field_array_;
 
+  struct BeforeExitCallback {
+    void (*cb_)(void* arg);
+    void* arg_;
+  };
+  std::list<BeforeExitCallback> before_exit_functions_;
+
   struct AtExitCallback {
     void (*cb_)(void* arg);
     void* arg_;
diff --git a/src/node.cc b/src/node.cc
index 95be389ce9456d..bb6edc5ed9657c 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -4555,6 +4555,14 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) {
 }
 
 
+void RunBeforeExit(Environment* env) {
+  env->RunBeforeExitCallbacks();
+
+  if (!uv_loop_alive(env->event_loop()))
+    EmitBeforeExit(env);
+}
+
+
 void EmitBeforeExit(Environment* env) {
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
@@ -4705,7 +4713,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
       if (more)
         continue;
 
-      EmitBeforeExit(&env);
+      RunBeforeExit(&env);
 
       // Emit `beforeExit` if the loop became alive either after emitting
       // event, or after running some callbacks.