diff --git a/benchmark/async_hooks/gc-tracking.js b/benchmark/async_hooks/gc-tracking.js new file mode 100644 index 00000000000000..c71c1b07aa5431 --- /dev/null +++ b/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'); + } +} diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 000982eb32af8d..41352820816b3f 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -545,12 +545,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(); @@ -568,11 +570,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: diff --git a/lib/async_hooks.js b/lib/async_hooks.js index afd3f5d85fea58..b642ec1a82f094 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -30,6 +30,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. @@ -260,13 +263,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', @@ -275,10 +287,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() { @@ -292,6 +310,7 @@ class AsyncResource { } emitDestroy() { + this[destroyedSymbol].destroyed = true; emitDestroyScript(this[async_id_symbol]); return this; } diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 6281a204accd4a..a3ef0d07e8d896 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -428,6 +428,46 @@ static void DisablePromiseHook(const FunctionCallbackInfo& args) { } +class DestroyParam { + public: + double asyncId; + v8::Persistent target; + v8::Persistent propBag; +}; + + +void AsyncWrap::WeakCallback(const v8::WeakCallbackInfo& info) { + HandleScope scope(info.GetIsolate()); + + Environment* env = Environment::GetCurrent(info.GetIsolate()); + DestroyParam* p = info.GetParameter(); + Local prop_bag = PersistentToLocal(info.GetIsolate(), p->propBag); + + Local 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& 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()->Value(); + p->target.Reset(isolate, args[0].As()); + p->propBag.Reset(isolate, args[2].As()); + p->target.SetWeak( + p, AsyncWrap::WeakCallback, v8::WeakCallbackType::kParameter); +} + + void AsyncWrap::GetAsyncId(const FunctionCallbackInfo& args) { AsyncWrap* wrap; args.GetReturnValue().Set(-1); @@ -503,6 +543,7 @@ void AsyncWrap::Initialize(Local 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::ReadOnly | v8::DontDelete); diff --git a/src/async-wrap.h b/src/async-wrap.h index 480a06e02d45bf..e6e717c24ed71d 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -86,6 +86,7 @@ namespace node { NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) class Environment; +class DestroyParam; class AsyncWrap : public BaseObject { public: @@ -165,6 +166,8 @@ class AsyncWrap : public BaseObject { virtual size_t self_size() const = 0; + static void WeakCallback(const v8::WeakCallbackInfo &info); + private: friend class PromiseWrap; diff --git a/src/env.h b/src/env.h index e26efd8f47bd39..93f18e8cbc5a61 100644 --- a/src/env.h +++ b/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(disposed_string, "_disposed") \ V(dns_a_string, "A") \ diff --git a/test/async-hooks/test-embedder.api.async-resource.js b/test/async-hooks/test-embedder.api.async-resource.js index f4dfba89557871..eeeaa447c9668c 100644 --- a/test/async-hooks/test-embedder.api.async-resource.js +++ b/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, diff --git a/test/parallel/test-async-hooks-destroy-on-gc.js b/test/parallel/test-async-hooks-destroy-on-gc.js new file mode 100644 index 00000000000000..fe6325e189734b --- /dev/null +++ b/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))); +}); diff --git a/test/parallel/test-async-hooks-disable-gc-tracking.js b/test/parallel/test-async-hooks-disable-gc-tracking.js new file mode 100644 index 00000000000000..a34739a9bb2b95 --- /dev/null +++ b/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(); + }); +}); diff --git a/test/parallel/test-async-hooks-prevent-double-destroy.js b/test/parallel/test-async-hooks-prevent-double-destroy.js new file mode 100644 index 00000000000000..5cd9c5e9a017cb --- /dev/null +++ b/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(); + }); +});