[async_wrap] backport of gc abort on destroy() fix on v6.x

src/async-wrap-inl.h

@@ -15,77 +15,6 @@
 
 namespace node {
 
-inline AsyncWrap::AsyncWrap(Environment* env,
-                            v8::Local<v8::Object> object,
-                            ProviderType provider,
-                            AsyncWrap* parent)
-    : BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1),
-      uid_(env->get_async_wrap_uid()) {
-  CHECK_NE(provider, PROVIDER_NONE);
-  CHECK_GE(object->InternalFieldCount(), 1);
-
-  // Shift provider value over to prevent id collision.
-  persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
-
-  v8::Local<v8::Function> init_fn = env->async_hooks_init_function();
-
-  // No init callback exists, no reason to go on.
-  if (init_fn.IsEmpty())
-    return;
-
-  // If async wrap callbacks are disabled and no parent was passed that has
-  // run the init callback then return.
-  if (!env->async_wrap_callbacks_enabled() &&
-      (parent == nullptr || !parent->ran_init_callback()))
-    return;
-
-  v8::HandleScope scope(env->isolate());
-
-  v8::Local<v8::Value> argv[] = {
-    v8::Number::New(env->isolate(), get_uid()),
-    v8::Int32::New(env->isolate(), provider),
-    Null(env->isolate()),
-    Null(env->isolate())
-  };
-
-  if (parent != nullptr) {
-    argv[2] = v8::Number::New(env->isolate(), parent->get_uid());
-    argv[3] = parent->object();
-  }
-
-  v8::TryCatch try_catch(env->isolate());
-
-  v8::MaybeLocal<v8::Value> ret =
-      init_fn->Call(env->context(), object, arraysize(argv), argv);
-
-  if (ret.IsEmpty()) {
-    ClearFatalExceptionHandlers(env);
-    FatalException(env->isolate(), try_catch);
-  }
-
-  bits_ |= 1;  // ran_init_callback() is true now.
-}
-
-
-inline AsyncWrap::~AsyncWrap() {
-  if (!ran_init_callback())
-    return;
-
-  v8::Local<v8::Function> fn = env()->async_hooks_destroy_function();
-  if (!fn.IsEmpty()) {
-    v8::HandleScope scope(env()->isolate());
-    v8::Local<v8::Value> uid = v8::Number::New(env()->isolate(), get_uid());
-    v8::TryCatch try_catch(env()->isolate());
-    v8::MaybeLocal<v8::Value> ret =
-        fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid);
-    if (ret.IsEmpty()) {
-      ClearFatalExceptionHandlers(env());
-      FatalException(env()->isolate(), try_catch);
-    }
-  }
-}
-
-
 inline bool AsyncWrap::ran_init_callback() const {
   return static_cast<bool>(bits_ & 1);
 }

src/async-wrap.cc

@@ -5,6 +5,7 @@
 #include "util.h"
 #include "util-inl.h"
 
+#include "uv.h"
 #include "v8.h"
 #include "v8-profiler.h"
 
@@ -14,6 +15,7 @@ using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::HeapProfiler;
+using v8::Int32;
 using v8::Integer;
 using v8::Isolate;
 using v8::Local;
@@ -155,9 +157,9 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-static void Initialize(Local<Object> target,
-                Local<Value> unused,
-                Local<Context> context) {
+void AsyncWrap::Initialize(Local<Object> target,
+                           Local<Value> unused,
+                           Local<Context> context) {
   Environment* env = Environment::GetCurrent(context);
   Isolate* isolate = env->isolate();
   HandleScope scope(isolate);
@@ -181,6 +183,38 @@ static void Initialize(Local<Object> target,
 }
 
 
+void AsyncWrap::DestroyIdsCb(uv_idle_t* handle) {
+  uv_idle_stop(handle);
+
+  Environment* env = Environment::from_destroy_ids_idle_handle(handle);
+  // None of the V8 calls done outside the HandleScope leak a handle. If this
+  // changes in the future then the SealHandleScope wrapping the uv_run()
+  // will catch this can cause the process to abort.
+  HandleScope handle_scope(env->isolate());
+  Context::Scope context_scope(env->context());
+  Local<Function> fn = env->async_hooks_destroy_function();
+
+  if (fn.IsEmpty())
+    return env->destroy_ids_list()->clear();
+
+  TryCatch try_catch(env->isolate());
+
+  for (auto current_id : *env->destroy_ids_list()) {
+    // Want each callback to be cleaned up after itself, instead of cleaning
+    // them all up after the while() loop completes.
+    HandleScope scope(env->isolate());
+    Local<Value> argv = Number::New(env->isolate(), current_id);
+    MaybeLocal<Value> ret = fn->Call(
+        env->context(), Undefined(env->isolate()), 1, &argv);
+
+    if (ret.IsEmpty()) {
+      ClearFatalExceptionHandlers(env);
+      FatalException(env->isolate(), try_catch);
+    }
+  }
+}
+
+
 void LoadAsyncWrapperInfo(Environment* env) {
   HeapProfiler* heap_profiler = env->isolate()->GetHeapProfiler();
 #define V(PROVIDER)                                                           \
@@ -191,6 +225,69 @@ void LoadAsyncWrapperInfo(Environment* env) {
 }
 
 
+AsyncWrap::AsyncWrap(Environment* env,
+                     Local<Object> object,
+                     ProviderType provider,
+                     AsyncWrap* parent)
+    : BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1),
+      uid_(env->get_async_wrap_uid()) {
+  CHECK_NE(provider, PROVIDER_NONE);
+  CHECK_GE(object->InternalFieldCount(), 1);
+
+  // Shift provider value over to prevent id collision.
+  persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
+
+  Local<Function> init_fn = env->async_hooks_init_function();
+
+  // No init callback exists, no reason to go on.
+  if (init_fn.IsEmpty())
+    return;
+
+  // If async wrap callbacks are disabled and no parent was passed that has
+  // run the init callback then return.
+  if (!env->async_wrap_callbacks_enabled() &&
+      (parent == nullptr || !parent->ran_init_callback()))
+    return;
+
+  HandleScope scope(env->isolate());
+
+  Local<Value> argv[] = {
+    Number::New(env->isolate(), get_uid()),
+    Int32::New(env->isolate(), provider),
+    Null(env->isolate()),
+    Null(env->isolate())
+  };
+
+  if (parent != nullptr) {
+    argv[2] = Number::New(env->isolate(), parent->get_uid());
+    argv[3] = parent->object();
+  }
+
+  TryCatch try_catch(env->isolate());
+
+  MaybeLocal<Value> ret =
+      init_fn->Call(env->context(), object, arraysize(argv), argv);
+
+  if (ret.IsEmpty()) {
+    ClearFatalExceptionHandlers(env);
+    FatalException(env->isolate(), try_catch);
+  }
+
+  bits_ |= 1;  // ran_init_callback() is true now.
+}
+
+
+AsyncWrap::~AsyncWrap() {
+  if (!ran_init_callback())
+    return;
+
+  if (env()->destroy_ids_list()->empty())
+    uv_idle_start(env()->destroy_ids_idle_handle(), DestroyIdsCb);
+
+  env()->destroy_ids_list()->push_back(get_uid());
+}
+
+
 Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
                                      int argc,
                                      Local<Value>* argv) {
@@ -290,4 +387,4 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
 
 }  // namespace node
 
-NODE_MODULE_CONTEXT_AWARE_BUILTIN(async_wrap, node::Initialize)
+NODE_MODULE_CONTEXT_AWARE_BUILTIN(async_wrap, node::AsyncWrap::Initialize)

src/async-wrap.h

@@ -4,6 +4,7 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include "base-object.h"
+#include "uv.h"
 #include "v8.h"
 
 #include <stdint.h>
@@ -49,12 +50,18 @@ class AsyncWrap : public BaseObject {
 #undef V
   };
 
-  inline AsyncWrap(Environment* env,
-                   v8::Local<v8::Object> object,
-                   ProviderType provider,
-                   AsyncWrap* parent = nullptr);
+  AsyncWrap(Environment* env,
+            v8::Local<v8::Object> object,
+            ProviderType provider,
+            AsyncWrap* parent = nullptr);
 
-  inline virtual ~AsyncWrap();
+  virtual ~AsyncWrap();
+
+  static void Initialize(v8::Local<v8::Object> target,
+                         v8::Local<v8::Value> unused,
+                         v8::Local<v8::Context> context);
+
+  static void DestroyIdsCb(uv_idle_t* handle);
 
   inline ProviderType provider_type() const;
 

src/env-inl.h

@@ -245,6 +245,8 @@ inline Environment::Environment(v8::Local<v8::Context> context,
 
   RB_INIT(&cares_task_list_);
   handle_cleanup_waiting_ = 0;
+
+  destroy_ids_list_.reserve(512);
 }
 
 inline Environment::~Environment() {
@@ -305,6 +307,15 @@ inline uv_idle_t* Environment::immediate_idle_handle() {
   return &immediate_idle_handle_;
 }
 
+inline Environment* Environment::from_destroy_ids_idle_handle(
+    uv_idle_t* handle) {
+  return ContainerOf(&Environment::destroy_ids_idle_handle_, handle);
+}
+
+inline uv_idle_t* Environment::destroy_ids_idle_handle() {
+  return &destroy_ids_idle_handle_;
+}
+
 inline Environment* Environment::from_idle_prepare_handle(
     uv_prepare_t* handle) {
   return ContainerOf(&Environment::idle_prepare_handle_, handle);
@@ -381,6 +392,10 @@ inline int64_t Environment::get_async_wrap_uid() {
   return ++async_wrap_uid_;
 }
 
+inline std::vector<int64_t>* Environment::destroy_ids_list() {
+  return &destroy_ids_list_;
+}
+
 inline uint32_t* Environment::heap_statistics_buffer() const {
   CHECK_NE(heap_statistics_buffer_, nullptr);
   return heap_statistics_buffer_;

src/env.h

@@ -16,6 +16,7 @@
 #include "v8.h"
 
 #include <stdint.h>
+#include <vector>
 
 // Caveat emptor: we're going slightly crazy with macros here but the end
 // hopefully justifies the means. We have a lot of per-context properties
@@ -413,8 +414,10 @@ class Environment {
   inline uint32_t watched_providers() const;
 
   static inline Environment* from_immediate_check_handle(uv_check_t* handle);
+  static inline Environment* from_destroy_ids_idle_handle(uv_idle_t* handle);
   inline uv_check_t* immediate_check_handle();
   inline uv_idle_t* immediate_idle_handle();
+  inline uv_idle_t* destroy_ids_idle_handle();
 
   static inline Environment* from_idle_prepare_handle(uv_prepare_t* handle);
   inline uv_prepare_t* idle_prepare_handle();
@@ -451,6 +454,9 @@ class Environment {
 
   inline int64_t get_async_wrap_uid();
 
+  // List of id's that have been destroyed and need the destroy() cb called.
+  inline std::vector<int64_t>* destroy_ids_list();
+
   inline uint32_t* heap_statistics_buffer() const;
   inline void set_heap_statistics_buffer(uint32_t* pointer);
 
@@ -543,6 +549,7 @@ class Environment {
   IsolateData* const isolate_data_;
   uv_check_t immediate_check_handle_;
   uv_idle_t immediate_idle_handle_;
+  uv_idle_t destroy_ids_idle_handle_;
   uv_prepare_t idle_prepare_handle_;
   uv_check_t idle_check_handle_;
   AsyncHooks async_hooks_;
@@ -558,6 +565,7 @@ class Environment {
   bool trace_sync_io_;
   size_t makecallback_cntr_;
   int64_t async_wrap_uid_;
+  std::vector<int64_t> destroy_ids_list_;
   debugger::Agent debugger_agent_;
 #if HAVE_INSPECTOR
   inspector::Agent inspector_agent_;

src/node.cc

@@ -4511,6 +4511,9 @@ Environment* CreateEnvironment(Isolate* isolate,
   uv_unref(reinterpret_cast<uv_handle_t*>(env->idle_prepare_handle()));
   uv_unref(reinterpret_cast<uv_handle_t*>(env->idle_check_handle()));
 
+  uv_idle_init(env->event_loop(), env->destroy_ids_idle_handle());
+  uv_unref(reinterpret_cast<uv_handle_t*>(env->destroy_ids_idle_handle()));
+
   // Register handle cleanups
   env->RegisterHandleCleanup(
       reinterpret_cast<uv_handle_t*>(env->immediate_check_handle()),

test/parallel/test-async-wrap-throw-from-callback.js

@@ -6,7 +6,7 @@ const assert = require('assert');
 const crypto = require('crypto');
 const domain = require('domain');
 const spawn = require('child_process').spawn;
-const callbacks = [ 'init', 'pre', 'post', 'destroy' ];
+const callbacks = [ 'init', 'pre', 'post' ];
 const toCall = process.argv[2];
 var msgCalled = 0;
 var msgReceived = 0;
@@ -23,13 +23,9 @@ function post() {
   if (toCall === 'post')
     throw new Error('post');
 }
-function destroy() {
-  if (toCall === 'destroy')
-    throw new Error('destroy');
-}
 
 if (typeof process.argv[2] === 'string') {
-  async_wrap.setupHooks({ init, pre, post, destroy });
+  async_wrap.setupHooks({ init, pre, post });
   async_wrap.enable();
 
   process.on('uncaughtException', () => assert.ok(0, 'UNREACHABLE'));