From 71ee15d34099826d8af999fc4d3690c885a7bce7 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Mon, 10 Jul 2017 14:28:33 +0200 Subject: [PATCH] async_hooks: make AsyncResource match emitInit AsyncResource previously called emitInitNative. Since AsyncResource is just an abstraction on top of the emitEventScript functions, it should call emitInitScript instead. PR-URL: https://github.com/nodejs/node/pull/14152 Reviewed-By: Refael Ackermann Reviewed-By: Trevor Norris Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- lib/async_hooks.js | 27 +++++++------------ .../test-embedder.api.async-resource.js | 4 +-- ...-async-hooks-asyncresource-constructor.js} | 8 +++++- 3 files changed, 18 insertions(+), 21 deletions(-) rename test/parallel/{test-async-wrap-asyncresource-constructor.js => test-async-hooks-asyncresource-constructor.js} (76%) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 7758c8d30577a0..39fd319fe54fe0 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -207,28 +207,16 @@ function triggerAsyncId() { // Embedder API // class AsyncResource { - constructor(type, triggerAsyncId) { - this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; - // Read and reset the current kInitTriggerId so that when the constructor - // finishes the kInitTriggerId field is always 0. - if (triggerAsyncId === undefined) { - triggerAsyncId = initTriggerId(); - // If a triggerAsyncId was passed, any kInitTriggerId still must be null'd. - } else { - async_uid_fields[kInitTriggerId] = 0; - } - this[trigger_id_symbol] = triggerAsyncId; - - if (typeof type !== 'string' || type.length <= 0) - throw new TypeError('type must be a string with length > 0'); + constructor(type, triggerAsyncId = initTriggerId()) { + // Unlike emitInitScript, AsyncResource doesn't supports null as the + // triggerAsyncId. if (!Number.isSafeInteger(triggerAsyncId) || triggerAsyncId < 0) throw new RangeError('triggerAsyncId must be an unsigned integer'); - // Return immediately if there's nothing to do. - if (async_hook_fields[kInit] === 0) - return; + this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; + this[trigger_id_symbol] = triggerAsyncId; - emitInitNative(this[async_id_symbol], type, triggerAsyncId, this); + emitInitScript(this[async_id_symbol], type, this[trigger_id_symbol], this); } emitBefore() { @@ -323,6 +311,9 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { // manually means that the embedder must have used initTriggerId(). if (triggerAsyncId === null) { triggerAsyncId = initTriggerId(); + } else { + // If a triggerAsyncId was passed, any kInitTriggerId still must be null'd. + async_uid_fields[kInitTriggerId] = 0; } // TODO(trevnorris): I'd prefer allowing these checks to not exist, or only diff --git a/test/async-hooks/test-embedder.api.async-resource.js b/test/async-hooks/test-embedder.api.async-resource.js index 6574db8fffe1b6..2aec828c73892d 100644 --- a/test/async-hooks/test-embedder.api.async-resource.js +++ b/test/async-hooks/test-embedder.api.async-resource.js @@ -18,8 +18,8 @@ assert.throws(() => new AsyncResource('invalid_trigger_id', null), /^RangeError: triggerAsyncId must be an unsigned integer$/); assert.strictEqual( - typeof new AsyncResource('default_trigger_id').triggerAsyncId(), - 'number' + new AsyncResource('default_trigger_id').triggerAsyncId(), + async_hooks.executionAsyncId() ); // create first custom event 'alcazares' with triggerAsyncId derived diff --git a/test/parallel/test-async-wrap-asyncresource-constructor.js b/test/parallel/test-async-hooks-asyncresource-constructor.js similarity index 76% rename from test/parallel/test-async-wrap-asyncresource-constructor.js rename to test/parallel/test-async-hooks-asyncresource-constructor.js index 2465f9590735ae..2fb0bb14ccf64e 100644 --- a/test/parallel/test-async-wrap-asyncresource-constructor.js +++ b/test/parallel/test-async-hooks-asyncresource-constructor.js @@ -4,7 +4,13 @@ require('../common'); // This tests that AsyncResource throws an error if bad parameters are passed const assert = require('assert'); -const AsyncResource = require('async_hooks').AsyncResource; +const async_hooks = require('async_hooks'); +const { AsyncResource } = async_hooks; + +// Setup init hook such parameters are validated +async_hooks.createHook({ + init() {} +}).enable(); assert.throws(() => { return new AsyncResource();