From 662c80bdae51ce2fda41981ddfebafb9a677fc49 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 15 Jul 2017 19:24:59 -0400 Subject: [PATCH 1/6] async-hooks: internalize `setInitTriggerId` * replace `setInitTriggerId` with triggerIdScopeSync --- lib/async_hooks.js | 58 ++++++++++++------------------------- lib/dgram.js | 20 ++++++++----- lib/internal/async_hooks.js | 53 +++++++++++++++++++++++++++++++++ lib/net.js | 45 ++++++++++++++++------------ 4 files changed, 109 insertions(+), 67 deletions(-) create mode 100644 lib/internal/async_hooks.js diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 39fd319fe54fe0..9bfbf733a9a542 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -1,25 +1,14 @@ 'use strict'; const internalUtil = require('internal/util'); -const async_wrap = process.binding('async_wrap'); -/* Both these arrays are used to communicate between JS and C++ with as little - * overhead as possible. - * - * async_hook_fields is a Uint32Array() that communicates the number of each - * type of active hooks of each type and wraps the uin32_t array of - * node::Environment::AsyncHooks::fields_. - * - * async_uid_fields is a Float64Array() that contains the async/trigger ids for - * several operations. These fields are as follows: - * kCurrentAsyncId: The async id of the current execution stack. - * kCurrentTriggerId: The trigger id of the current execution stack. - * kAsyncUidCntr: Counter that tracks the unique ids given to new resources. - * kInitTriggerId: Written to just before creating a new resource, so the - * constructor knows what other resource is responsible for its init(). - * Used this way so the trigger id doesn't need to be passed to every - * resource's constructor. - */ -const { async_hook_fields, async_uid_fields } = async_wrap; +const { + async_wrap, + async_hook_fields, + async_uid_fields, + executionAsyncId, + triggerAsyncId +} = require('internal/async_hooks'); + // Used to change the state of the async id stack. const { pushAsyncIds, popAsyncIds } = async_wrap; // Array of all AsyncHooks that will be iterated whenever an async event fires. @@ -39,8 +28,15 @@ var tmp_async_hook_fields = null; // Each constant tracks how many callbacks there are for any given step of // async execution. These are tracked so if the user didn't include callbacks // for a given step, that step can bail out early. -const { kInit, kBefore, kAfter, kDestroy, kTotals, kCurrentAsyncId, - kCurrentTriggerId, kAsyncUidCntr, kInitTriggerId } = async_wrap.constants; +const { + kInit, + kBefore, + kAfter, + kDestroy, + kTotals, + kAsyncUidCntr, + kInitTriggerId +} = async_wrap.constants; const { async_id_symbol, trigger_id_symbol } = async_wrap; @@ -194,16 +190,6 @@ function createHook(fns) { } -function executionAsyncId() { - return async_uid_fields[kCurrentAsyncId]; -} - - -function triggerAsyncId() { - return async_uid_fields[kCurrentTriggerId]; -} - - // Embedder API // class AsyncResource { @@ -287,18 +273,11 @@ function initTriggerId() { // inherit it by accident. async_uid_fields[kInitTriggerId] = 0; if (tId <= 0) - tId = async_uid_fields[kCurrentAsyncId]; + tId = executionAsyncId(); return tId; } -function setInitTriggerId(triggerAsyncId) { - // CHECK(Number.isSafeInteger(triggerAsyncId)) - // CHECK(triggerAsyncId > 0) - async_uid_fields[kInitTriggerId] = triggerAsyncId; -} - - function emitInitScript(asyncId, type, triggerAsyncId, resource) { // Short circuit all checks for the common case. Which is that no hooks have // been set. Do this to remove performance impact for embedders (and core). @@ -455,7 +434,6 @@ module.exports = { // Sensitive Embedder API newUid, initTriggerId, - setInitTriggerId, emitInit: emitInitScript, emitBefore: emitBeforeScript, emitAfter: emitAfterScript, diff --git a/lib/dgram.js b/lib/dgram.js index 3204ad87cd5cd9..2b427c14387544 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -26,7 +26,7 @@ const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; const util = require('util'); const EventEmitter = require('events'); -const setInitTriggerId = require('async_hooks').setInitTriggerId; +const { triggerIdScopeSync } = require('internal/async_hooks'); const UV_UDP_REUSEADDR = process.binding('constants').os.UV_UDP_REUSEADDR; const async_id_symbol = process.binding('async_wrap').async_id_symbol; const nextTick = require('internal/process/next_tick').nextTick; @@ -455,13 +455,17 @@ function doSend(ex, self, ip, list, address, port, callback) { // node::SendWrap isn't instantiated and attached to the JS instance of // SendWrap above until send() is called. So don't set the init trigger id // until now. - setInitTriggerId(self[async_id_symbol]); - var err = self._handle.send(req, - list, - list.length, - port, - ip, - !!callback); + const err = triggerIdScopeSync( + self[async_id_symbol], + () => self._handle.send( + req, + list, + list.length, + port, + ip, + !!callback + ) + ); if (err && callback) { // don't emit as error, dgram_legacy.js compatibility const ex = exceptionWithHostPort(err, 'send', address, port); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js new file mode 100644 index 00000000000000..b0869369e1f9a6 --- /dev/null +++ b/lib/internal/async_hooks.js @@ -0,0 +1,53 @@ +'use strict'; + +const async_wrap = process.binding('async_wrap'); + +/* Both these arrays are used to communicate between JS and C++ with as little + * overhead as possible. + * + * async_hook_fields is a Uint32Array() that communicates the number of each + * type of active hooks of each type and wraps the uin32_t array of + * node::Environment::AsyncHooks::fields_. + * + * async_uid_fields is a Float64Array() that contains the async/trigger ids for + * several operations. These fields are as follows: + * kCurrentAsyncId: The async id of the current execution stack. + * kCurrentTriggerId: The trigger id of the current execution stack. + * kAsyncUidCntr: Counter that tracks the unique ids given to new resources. + * kInitTriggerId: Written to just before creating a new resource, so the + * constructor knows what other resource is responsible for its init(). + * Used this way so the trigger id doesn't need to be passed to every + * resource's constructor. + */ +const { async_hook_fields, async_uid_fields } = async_wrap; +const { kCurrentTriggerId, kCurrentAsyncId } = async_wrap.constants; + +function executionAsyncId() { + return async_uid_fields[kCurrentAsyncId]; +} + + +function triggerAsyncId() { + return async_uid_fields[kCurrentTriggerId]; +} + + +function triggerIdScopeSync(id, block) { + const old = async_uid_fields[kCurrentTriggerId]; + async_uid_fields[kCurrentTriggerId] = id; + try { + return block(); + } finally { + async_uid_fields[kCurrentTriggerId] = old; + } +} + + +module.export = { + async_wrap, + async_hook_fields, + async_uid_fields, + executionAsyncId, + triggerAsyncId, + triggerIdScopeSync, +}; diff --git a/lib/net.js b/lib/net.js index 5129a596421e1c..09d36cac18870d 100644 --- a/lib/net.js +++ b/lib/net.js @@ -40,7 +40,8 @@ const PipeConnectWrap = process.binding('pipe_wrap').PipeConnectWrap; const ShutdownWrap = process.binding('stream_wrap').ShutdownWrap; const WriteWrap = process.binding('stream_wrap').WriteWrap; const async_id_symbol = process.binding('async_wrap').async_id_symbol; -const { newUid, setInitTriggerId } = require('async_hooks'); +const { triggerIdScopeSync } = require('internal/async_hooks'); +const { newUid } = require('async_hooks'); const nextTick = require('internal/process/next_tick').nextTick; const errors = require('internal/errors'); @@ -290,10 +291,12 @@ function onSocketFinish() { req.oncomplete = afterShutdown; req.handle = this._handle; // node::ShutdownWrap isn't instantiated and attached to the JS instance of - // ShutdownWrap above until shutdown() is called. So don't set the init + // ShutdownWrap above until shutdown() is called. So we don't set the init // trigger id until now. - setInitTriggerId(this[async_id_symbol]); - var err = this._handle.shutdown(req); + const err = triggerIdScopeSync( + this[async_id_symbol], + () => this._handle.shutdown(req) + ); if (err) return this.destroy(errnoException(err, 'shutdown')); @@ -904,23 +907,24 @@ function internalConnect( req.localPort = localPort; // node::TCPConnectWrap isn't instantiated and attached to the JS instance - // of TCPConnectWrap above until connect() is called. So don't set the init - // trigger id until now. - setInitTriggerId(self[async_id_symbol]); - if (addressType === 4) - err = self._handle.connect(req, address, port); - else - err = self._handle.connect6(req, address, port); - + // of TCPConnectWrap above until connect() is called. So we don't set the + // init triggerID until now. + const methodName = addressType === 4 ? 'connect' : 'connect6'; + err = triggerIdScopeSync( + self[async_id_symbol], + () => self._handle[methodName](req, address, port) + ); } else { const req = new PipeConnectWrap(); req.address = address; req.oncomplete = afterConnect; // node::PipeConnectWrap isn't instantiated and attached to the JS instance - // of PipeConnectWrap above until connect() is called. So don't set the - // init trigger id until now. - setInitTriggerId(self[async_id_symbol]); - err = self._handle.connect(req, address, afterConnect); + // of PipeConnectWrap above until connect() is called. So we don't set the + // init triggerID until now. + err = triggerIdScopeSync( + self[async_id_symbol], + () => self._handle.connect(req, address, afterConnect) + ); } if (err) { @@ -1051,8 +1055,7 @@ function lookupAndConnect(self, options) { debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; - setInitTriggerId(self[async_id_symbol]); - lookup(host, dnsopts, function emitLookup(err, ip, addressType) { + function emitLookup(err, ip, addressType) { self.emit('lookup', err, ip, addressType, host); // It's possible we were destroyed while looking this up. @@ -1078,7 +1081,11 @@ function lookupAndConnect(self, options) { localAddress, localPort); } - }); + } + triggerIdScopeSync( + self[async_id_symbol], + lookup(host, dnsopts, emitLookup) + ); } From 76cdbfef21b5c267322b9d0825102031f70c80e2 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 15 Jul 2017 19:46:50 -0400 Subject: [PATCH 2/6] [fixup] add to node.gyp --- node.gyp | 1 + 1 file changed, 1 insertion(+) diff --git a/node.gyp b/node.gyp index b0e4676a96e477..5369ea6cb0aa72 100644 --- a/node.gyp +++ b/node.gyp @@ -74,6 +74,7 @@ 'lib/v8.js', 'lib/vm.js', 'lib/zlib.js', + 'lib/internal/async_hooks.js', 'lib/internal/buffer.js', 'lib/internal/child_process.js', 'lib/internal/cluster/child.js', From 130ab0a48931d48a4ca5715a8dfec07df9197927 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 15 Jul 2017 20:12:52 -0400 Subject: [PATCH 3/6] [fixup] typo --- lib/internal/async_hooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index b0869369e1f9a6..7c10764a1bdba6 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -43,7 +43,7 @@ function triggerIdScopeSync(id, block) { } -module.export = { +module.exports = { async_wrap, async_hook_fields, async_uid_fields, From 8414756a18a7acc7ca1f30485e6ec446b47886c0 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 15 Jul 2017 20:43:24 -0400 Subject: [PATCH 4/6] [fixup] func not call --- lib/net.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index 09d36cac18870d..8685be93acca7f 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1084,7 +1084,7 @@ function lookupAndConnect(self, options) { } triggerIdScopeSync( self[async_id_symbol], - lookup(host, dnsopts, emitLookup) + () => lookup(host, dnsopts, emitLookup) ); } From 605de08e6e4f02564d9e3a00ee6c6a9ac2a88f12 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sun, 16 Jul 2017 06:46:05 -0400 Subject: [PATCH 5/6] [fixup] `restore` iff `retrieve` --- lib/internal/async_hooks.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 7c10764a1bdba6..587e4d1b71549e 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -33,12 +33,14 @@ function triggerAsyncId() { function triggerIdScopeSync(id, block) { - const old = async_uid_fields[kCurrentTriggerId]; + let old = async_uid_fields[kCurrentTriggerId]; async_uid_fields[kCurrentTriggerId] = id; try { - return block(); + const ret = block(); + if (async_uid_fields[kCurrentTriggerId] !== id) old = null; + return ret; } finally { - async_uid_fields[kCurrentTriggerId] = old; + old && (async_uid_fields[kCurrentTriggerId] = old); } } From c20a114cd072fc2f573989362ea7360abb06bbef Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sun, 16 Jul 2017 07:22:42 -0400 Subject: [PATCH 6/6] [fixup] kInitTriggerId not kCurrentTriggerId (dyslexia FTW) --- lib/async_hooks.js | 22 ++++----------- lib/internal/async_hooks.js | 54 +++++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 9bfbf733a9a542..b74e657f9b4ff5 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -5,6 +5,9 @@ const { async_wrap, async_hook_fields, async_uid_fields, + internalJSConstants, + initTriggerId, + resetTriggerId, executionAsyncId, triggerAsyncId } = require('internal/async_hooks'); @@ -35,8 +38,7 @@ const { kDestroy, kTotals, kAsyncUidCntr, - kInitTriggerId -} = async_wrap.constants; +} = internalJSConstants; const { async_id_symbol, trigger_id_symbol } = async_wrap; @@ -264,20 +266,6 @@ function newUid() { } -// Return the triggerAsyncId meant for the constructor calling it. It's up to -// the user to safeguard this call and make sure it's zero'd out when the -// constructor is complete. -function initTriggerId() { - var tId = async_uid_fields[kInitTriggerId]; - // Reset value after it's been called so the next constructor doesn't - // inherit it by accident. - async_uid_fields[kInitTriggerId] = 0; - if (tId <= 0) - tId = executionAsyncId(); - return tId; -} - - function emitInitScript(asyncId, type, triggerAsyncId, resource) { // Short circuit all checks for the common case. Which is that no hooks have // been set. Do this to remove performance impact for embedders (and core). @@ -292,7 +280,7 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { triggerAsyncId = initTriggerId(); } else { // If a triggerAsyncId was passed, any kInitTriggerId still must be null'd. - async_uid_fields[kInitTriggerId] = 0; + resetTriggerId(); } // TODO(trevnorris): I'd prefer allowing these checks to not exist, or only diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 587e4d1b71549e..ba5c87aa29f7b7 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -20,7 +20,48 @@ const async_wrap = process.binding('async_wrap'); * resource's constructor. */ const { async_hook_fields, async_uid_fields } = async_wrap; -const { kCurrentTriggerId, kCurrentAsyncId } = async_wrap.constants; +const { + kCurrentTriggerId, + kCurrentAsyncId, + kInitTriggerId +} = async_wrap.constants; + +const { + kInit, + kBefore, + kAfter, + kDestroy, + kTotals, + kAsyncUidCntr, +} = async_wrap.constants; + +const internalJSConstants = { + kInit, + kBefore, + kAfter, + kDestroy, + kTotals, + kAsyncUidCntr, +}; + +// Return the triggerAsyncId meant for the constructor calling it. It's up to +// the user to safeguard this call and make sure it's zero'd out when the +// constructor is complete. +function initTriggerId() { + var tId = async_uid_fields[kInitTriggerId]; + // Reset value after it's been called so the next constructor doesn't + // inherit it by accident. + async_uid_fields[kInitTriggerId] = 0; + if (tId <= 0) + tId = executionAsyncId(); + return tId; +} + + +function resetTriggerId() { + async_uid_fields[kInitTriggerId] = 0; +} + function executionAsyncId() { return async_uid_fields[kCurrentAsyncId]; @@ -33,14 +74,14 @@ function triggerAsyncId() { function triggerIdScopeSync(id, block) { - let old = async_uid_fields[kCurrentTriggerId]; - async_uid_fields[kCurrentTriggerId] = id; + let old = async_uid_fields[kInitTriggerId]; + async_uid_fields[kInitTriggerId] = id; try { const ret = block(); - if (async_uid_fields[kCurrentTriggerId] !== id) old = null; + if (async_uid_fields[kInitTriggerId] !== id) old = null; return ret; } finally { - old && (async_uid_fields[kCurrentTriggerId] = old); + old && (async_uid_fields[kInitTriggerId] = old); } } @@ -49,6 +90,9 @@ module.exports = { async_wrap, async_hook_fields, async_uid_fields, + internalJSConstants, + initTriggerId, + resetTriggerId, executionAsyncId, triggerAsyncId, triggerIdScopeSync,