AsyncWrap external API improvements

src/async-wrap-inl.h

@@ -17,7 +17,8 @@ inline AsyncWrap::AsyncWrap(Environment* env,
                             v8::Local<v8::Object> object,
                             ProviderType provider,
                             AsyncWrap* parent)
-    : BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1) {
+    : 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);
 
@@ -40,11 +41,12 @@ inline AsyncWrap::AsyncWrap(Environment* env,
 
   v8::Local<v8::Value> argv[] = {
     v8::Int32::New(env->isolate(), provider),
+    v8::Integer::New(env->isolate(), get_uid()),
     Null(env->isolate())
   };
 
   if (parent != nullptr)
-    argv[1] = parent->object();
+    argv[2] = parent->object();
 
   v8::MaybeLocal<v8::Value> ret =
       init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv);
@@ -56,6 +58,22 @@ inline AsyncWrap::AsyncWrap(Environment* env,
 }
 
 
+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::Integer::New(env()->isolate(), get_uid());
+    v8::MaybeLocal<v8::Value> ret =
+        fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid);
+    if (ret.IsEmpty())
+      FatalError("node::AsyncWrap::~AsyncWrap", "destroy hook threw");
+  }
+}
+
+
 inline bool AsyncWrap::ran_init_callback() const {
   return static_cast<bool>(bits_ & 1);
 }
@@ -66,6 +84,11 @@ inline AsyncWrap::ProviderType AsyncWrap::provider_type() const {
 }
 
 
+inline int64_t AsyncWrap::get_uid() const {
+  return uid_;
+}
+
+
 inline v8::Local<v8::Value> AsyncWrap::MakeCallback(
     const v8::Local<v8::String> symbol,
     int argc,

src/async-wrap.cc

@@ -103,6 +103,9 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local<Value> wrapper) {
 
 static void EnableHooksJS(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  Local<Function> init_fn = env->async_hooks_init_function();
+  if (init_fn.IsEmpty() || !init_fn->IsFunction())
+    return env->ThrowTypeError("init callback is not assigned to a function");
   env->async_hooks()->set_enable_callbacks(1);
 }
 
@@ -116,13 +119,20 @@ static void DisableHooksJS(const FunctionCallbackInfo<Value>& args) {
 static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK(args[0]->IsFunction());
-  CHECK(args[1]->IsFunction());
-  CHECK(args[2]->IsFunction());
+  if (env->async_hooks()->callbacks_enabled())
+    return env->ThrowError("hooks should not be set while also enabled");
+
+  if (!args[0]->IsFunction())
+    return env->ThrowTypeError("init callback must be a function");
 
   env->set_async_hooks_init_function(args[0].As<Function>());
-  env->set_async_hooks_pre_function(args[1].As<Function>());
-  env->set_async_hooks_post_function(args[2].As<Function>());
+
+  if (args[1]->IsFunction())
+    env->set_async_hooks_pre_function(args[1].As<Function>());
+  if (args[2]->IsFunction())
+    env->set_async_hooks_post_function(args[2].As<Function>());
+  if (args[3]->IsFunction())
+    env->set_async_hooks_destroy_function(args[3].As<Function>());
 }
 
 
@@ -148,6 +158,7 @@ static void Initialize(Local<Object> target,
   env->set_async_hooks_init_function(Local<Function>());
   env->set_async_hooks_pre_function(Local<Function>());
   env->set_async_hooks_post_function(Local<Function>());
+  env->set_async_hooks_destroy_function(Local<Function>());
 }
 
 

src/async-wrap.h

@@ -51,10 +51,12 @@ class AsyncWrap : public BaseObject {
                    ProviderType provider,
                    AsyncWrap* parent = nullptr);
 
-  inline virtual ~AsyncWrap() override = default;
+  inline virtual ~AsyncWrap();
 
   inline ProviderType provider_type() const;
 
+  inline int64_t get_uid() const;
+
   // Only call these within a valid HandleScope.
   v8::Local<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
                                      int argc,
@@ -76,6 +78,7 @@ class AsyncWrap : public BaseObject {
   // expected the context object will receive a _asyncQueue object property
   // that will be used to call pre/post in MakeCallback.
   uint32_t bits_;
+  const int64_t uid_;
 };
 
 void LoadAsyncWrapperInfo(Environment* env);

src/env-inl.h

@@ -210,6 +210,7 @@ inline Environment::Environment(v8::Local<v8::Context> context,
       using_domains_(false),
       printed_error_(false),
       trace_sync_io_(false),
+      async_wrap_uid_(0),
       debugger_agent_(this),
       http_parser_buffer_(nullptr),
       context_(context->GetIsolate(), context) {
@@ -359,6 +360,10 @@ inline void Environment::set_trace_sync_io(bool value) {
   trace_sync_io_ = value;
 }
 
+inline int64_t Environment::get_async_wrap_uid() {
+  return ++async_wrap_uid_;
+}
+
 inline uint32_t* Environment::heap_statistics_buffer() const {
   CHECK_NE(heap_statistics_buffer_, nullptr);
   return heap_statistics_buffer_;

src/env.h

@@ -236,6 +236,7 @@ namespace node {
   V(async_hooks_init_function, v8::Function)                                  \
   V(async_hooks_pre_function, v8::Function)                                   \
   V(async_hooks_post_function, v8::Function)                                  \
+  V(async_hooks_destroy_function, v8::Function)                               \
   V(binding_cache_object, v8::Object)                                         \
   V(buffer_constructor_function, v8::Function)                                \
   V(buffer_prototype_object, v8::Object)                                      \
@@ -446,6 +447,8 @@ class Environment {
   void PrintSyncTrace() const;
   inline void set_trace_sync_io(bool value);
 
+  inline int64_t get_async_wrap_uid();
+
   bool KickNextTick();
 
   inline uint32_t* heap_statistics_buffer() const;
@@ -541,6 +544,7 @@ class Environment {
   bool using_domains_;
   bool printed_error_;
   bool trace_sync_io_;
+  int64_t async_wrap_uid_;
   debugger::Agent debugger_agent_;
 
   HandleWrapQueue handle_wrap_queue_;

test/parallel/test-async-wrap-disabled-propagate-parent.js

@@ -10,7 +10,7 @@ let cntr = 0;
 let server;
 let client;
 
-function init(type, parent) {
+function init(type, id, parent) {
   if (parent) {
     cntr++;
     // Cannot assert in init callback or will abort.

test/parallel/test-async-wrap-propagate-parent.js

@@ -9,7 +9,7 @@ let cntr = 0;
 let server;
 let client;
 
-function init(type, parent) {
+function init(type, id, parent) {
   if (parent) {
     cntr++;
     // Cannot assert in init callback or will abort.

test/parallel/test-async-wrap-throw-no-init.js

@@ -0,0 +1,22 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const async_wrap = process.binding('async_wrap');
+
+
+assert.throws(function() {
+  async_wrap.setupHooks(null);
+}, /init callback must be a function/);
+
+assert.throws(function() {
+  async_wrap.enable();
+}, /init callback is not assigned to a function/);
+
+// Should not throw
+async_wrap.setupHooks(() => {});
+async_wrap.enable();
+
+assert.throws(function() {
+  async_wrap.setupHooks(() => {});
+}, /hooks should not be set while also enabled/);