async_hooks: Add destroy event for gced AsyncResources

benchmark/async_hooks/gc-tracking.js

@@ -0,0 +1,45 @@
+'use strict';
+const common = require('../common.js');
+const { AsyncResource } = require('async_hooks');
+
+const bench = common.createBenchmark(main, {
+  n: [1e6],
+  method: [
+    'trackingEnabled',
+    'trackingDisabled',
+  ]
+}, {
+  flags: ['--expose-gc']
+});
+
+function endAfterGC(n) {
+  setImmediate(() => {
+    global.gc();
+    setImmediate(() => {
+      bench.end(n);
+    });
+  });
+}
+
+function main(conf) {
+  const n = +conf.n;
+
+  switch (conf.method) {
+    case 'trackingEnabled':
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        new AsyncResource('foobar');
+      }
+      endAfterGC(n);
+      break;
+    case 'trackingDisabled':
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        new AsyncResource('foobar', { requireManualDestroy: true });
+      }
+      endAfterGC(n);
+      break;
+    default:
+      throw new Error('Unsupported method');
+  }
+}

doc/api/async_hooks.md

@@ -543,12 +543,14 @@ will occur and the process will abort.
 The following is an overview of the `AsyncResource` API.
 
 ```js
-const { AsyncResource } = require('async_hooks');
+const { AsyncResource, executionAsyncId } = require('async_hooks');
 
 // AsyncResource() is meant to be extended. Instantiating a
 // new AsyncResource() also triggers init. If triggerAsyncId is omitted then
 // async_hook.executionAsyncId() is used.
-const asyncResource = new AsyncResource(type, triggerAsyncId);
+const asyncResource = new AsyncResource(
+  type, { triggerAsyncId: executionAsyncId(), requireManualDestroy: false }
+);
 
 // Call AsyncHooks before callbacks.
 asyncResource.emitBefore();
@@ -566,11 +568,17 @@ asyncResource.asyncId();
 asyncResource.triggerAsyncId();
 ```
 
-#### `AsyncResource(type[, triggerAsyncId])`
+#### `AsyncResource(type[, options])`
 
 * `type` {string} The type of async event.
-* `triggerAsyncId` {number} The ID of the execution context that created this
-  async event.
+* `options` {Object}
+  * `triggerAsyncId` {number} The ID of the execution context that created this
+  async event.  **Default:** `executionAsyncId()`
+  * `requireManualDestroy` {boolean} Disables automatic `emitDestroy` when the
+  object is garbage collected. This usually does not need to be set (even if
+  `emitDestroy` is called manually), unless the resource's asyncId is retrieved
+  and the sensitive API's `emitDestroy` is called with it.
+  **Default:** `false`
 
 Example usage:
 

lib/async_hooks.js

@@ -29,6 +29,9 @@ const { async_hook_fields, async_id_fields } = async_wrap;
 const { pushAsyncIds, popAsyncIds } = async_wrap;
 // For performance reasons, only track Proimses when a hook is enabled.
 const { enablePromiseHook, disablePromiseHook } = async_wrap;
+// For userland AsyncResources, make sure to emit a destroy event when the
+// resource gets gced.
+const { registerDestroyHook } = async_wrap;
 // Properties in active_hooks are used to keep track of the set of hooks being
 // executed in case another hook is enabled/disabled. The new set of hooks is
 // then restored once the active set of hooks is finished executing.
@@ -259,13 +262,22 @@ function validateAsyncId(asyncId, type) {
 
 // Embedder API //
 
+const destroyedSymbol = Symbol('destroyed');
+
 class AsyncResource {
-  constructor(type, triggerAsyncId = initTriggerId()) {
+  constructor(type, opts = {}) {
     if (typeof type !== 'string')
       throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'type', 'string');
 
+    if (typeof opts === 'number') {
+      opts = { triggerAsyncId: opts, requireManualDestroy: false };
+    } else if (opts.triggerAsyncId === undefined) {
+      opts.triggerAsyncId = initTriggerId();
+    }
+
     // Unlike emitInitScript, AsyncResource doesn't supports null as the
     // triggerAsyncId.
+    const triggerAsyncId = opts.triggerAsyncId;
     if (!Number.isSafeInteger(triggerAsyncId) || triggerAsyncId < -1) {
       throw new errors.RangeError('ERR_INVALID_ASYNC_ID',
                                   'triggerAsyncId',
@@ -274,10 +286,16 @@ class AsyncResource {
 
     this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
     this[trigger_async_id_symbol] = triggerAsyncId;
+    // this prop name (destroyed) has to be synchronized with C++
+    this[destroyedSymbol] = { destroyed: false };
 
     emitInitScript(
       this[async_id_symbol], type, this[trigger_async_id_symbol], this
     );
+
+    if (!opts.requireManualDestroy) {
+      registerDestroyHook(this, this[async_id_symbol], this[destroyedSymbol]);
+    }
   }
 
   emitBefore() {
@@ -291,6 +309,7 @@ class AsyncResource {
   }
 
   emitDestroy() {
+    this[destroyedSymbol].destroyed = true;
     emitDestroyScript(this[async_id_symbol]);
     return this;
   }

src/async-wrap.cc

@@ -427,6 +427,46 @@ static void DisablePromiseHook(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+class DestroyParam {
+ public:
+  double asyncId;
+  v8::Persistent<Object> target;
+  v8::Persistent<Object> propBag;
+};
+
+
+void AsyncWrap::WeakCallback(const v8::WeakCallbackInfo<DestroyParam>& info) {
+  HandleScope scope(info.GetIsolate());
+
+  Environment* env = Environment::GetCurrent(info.GetIsolate());
+  DestroyParam* p = info.GetParameter();
+  Local<Object> prop_bag = PersistentToLocal(info.GetIsolate(), p->propBag);
+
+  Local<Value> val = prop_bag->Get(env->destroyed_string());
+  if (val->IsFalse()) {
+    AsyncWrap::EmitDestroy(env, p->asyncId);
+  }
+  p->target.Reset();
+  p->propBag.Reset();
+  delete p;
+}
+
+
+static void RegisterDestroyHook(const FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsNumber());
+  CHECK(args[2]->IsObject());
+
+  Isolate* isolate = args.GetIsolate();
+  DestroyParam* p = new DestroyParam();
+  p->asyncId = args[1].As<Number>()->Value();
+  p->target.Reset(isolate, args[0].As<Object>());
+  p->propBag.Reset(isolate, args[2].As<Object>());
+  p->target.SetWeak(
+    p, AsyncWrap::WeakCallback, v8::WeakCallbackType::kParameter);
+}
+
+
 void AsyncWrap::GetAsyncId(const FunctionCallbackInfo<Value>& args) {
   AsyncWrap* wrap;
   args.GetReturnValue().Set(-1);
@@ -502,6 +542,7 @@ void AsyncWrap::Initialize(Local<Object> target,
   env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId);
   env->SetMethod(target, "enablePromiseHook", EnablePromiseHook);
   env->SetMethod(target, "disablePromiseHook", DisablePromiseHook);
+  env->SetMethod(target, "registerDestroyHook", RegisterDestroyHook);
 
   v8::PropertyAttribute ReadOnlyDontDelete =
       static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete);

src/async-wrap.h

@@ -84,6 +84,7 @@ namespace node {
   NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
 
 class Environment;
+class DestroyParam;
 
 class AsyncWrap : public BaseObject {
  public:
@@ -163,6 +164,8 @@ class AsyncWrap : public BaseObject {
 
   virtual size_t self_size() const = 0;
 
+  static void WeakCallback(const v8::WeakCallbackInfo<DestroyParam> &info);
+
  private:
   friend class PromiseWrap;
 

src/env.h

@@ -120,6 +120,7 @@ class ModuleWrap;
   V(cwd_string, "cwd")                                                        \
   V(dest_string, "dest")                                                      \
   V(destroy_string, "destroy")                                                \
+  V(destroyed_string, "destroyed")                                            \
   V(detached_string, "detached")                                              \
   V(dns_a_string, "A")                                                        \
   V(dns_aaaa_string, "AAAA")                                                  \

test/async-hooks/test-embedder.api.async-resource.js

@@ -18,7 +18,7 @@ common.expectsError(
     type: TypeError,
   });
 assert.throws(() => {
-  new AsyncResource('invalid_trigger_id', null);
+  new AsyncResource('invalid_trigger_id', { triggerAsyncId: null });
 }, common.expectsError({
   code: 'ERR_INVALID_ASYNC_ID',
   type: RangeError,

test/parallel/test-async-hooks-destroy-on-gc.js

@@ -0,0 +1,27 @@
+'use strict';
+// Flags: --expose_gc
+
+// This test ensures that userland-only AsyncResources cause a destroy event to
+// be emitted when they get gced.
+
+const common = require('../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+
+const destroyedIds = new Set();
+async_hooks.createHook({
+  destroy: common.mustCallAtLeast((asyncId) => {
+    destroyedIds.add(asyncId);
+  }, 1)
+}).enable();
+
+let asyncId = null;
+{
+  const res = new async_hooks.AsyncResource('foobar');
+  asyncId = res.asyncId();
+}
+
+setImmediate(() => {
+  global.gc();
+  setImmediate(() => assert.ok(destroyedIds.has(asyncId)));
+});

test/parallel/test-async-hooks-disable-gc-tracking.js

@@ -0,0 +1,21 @@
+'use strict';
+// Flags: --expose_gc
+
+// This test ensures that userland-only AsyncResources cause a destroy event to
+// be emitted when they get gced.
+
+const common = require('../common');
+const async_hooks = require('async_hooks');
+
+const hook = async_hooks.createHook({
+  destroy: common.mustCall(1) // only 1 immediate is destroyed
+}).enable();
+
+new async_hooks.AsyncResource('foobar', { requireManualDestroy: true });
+
+setImmediate(() => {
+  global.gc();
+  setImmediate(() => {
+    hook.disable();
+  });
+});

test/parallel/test-async-hooks-prevent-double-destroy.js

@@ -0,0 +1,24 @@
+'use strict';
+// Flags: --expose_gc
+
+// This test ensures that userland-only AsyncResources cause a destroy event to
+// be emitted when they get gced.
+
+const common = require('../common');
+const async_hooks = require('async_hooks');
+
+const hook = async_hooks.createHook({
+  destroy: common.mustCall(2) // 1 immediate + manual destroy
+}).enable();
+
+{
+  const res = new async_hooks.AsyncResource('foobar');
+  res.emitDestroy();
+}
+
+setImmediate(() => {
+  global.gc();
+  setImmediate(() => {
+    hook.disable();
+  });
+});