Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor async_hooks initTriggerId #17273

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ function responseKeepAlive(res, req) {
socket.removeListener('error', socketErrorListener);
socket.once('error', freeSocketErrorListener);
// There are cases where _handle === null. Avoid those. Passing null to
// nextTick() will call initTriggerId() to retrieve the id.
// nextTick() will call getDefaultTriggerAsyncId() to retrieve the id.
const asyncId = socket._handle ? socket._handle.getAsyncId() : null;
// Mark this socket as available, AFTER user-added end
// handlers have a chance to run.
Expand Down
4 changes: 2 additions & 2 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const {
disableHooks,
// Internal Embedder API
newUid,
initTriggerId,
getDefaultTriggerAsyncId,
emitInit,
emitBefore,
emitAfter,
Expand Down Expand Up @@ -147,7 +147,7 @@ class AsyncResource {
if (typeof opts === 'number') {
opts = { triggerAsyncId: opts, requireManualDestroy: false };
} else if (opts.triggerAsyncId === undefined) {
opts.triggerAsyncId = initTriggerId();
opts.triggerAsyncId = getDefaultTriggerAsyncId();
}

// Unlike emitInitScript, AsyncResource doesn't supports null as the
Expand Down
21 changes: 11 additions & 10 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const dns = require('dns');
const util = require('util');
const { isUint8Array } = require('internal/util/types');
const EventEmitter = require('events');
const { setInitTriggerId } = require('internal/async_hooks');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { UV_UDP_REUSEADDR } = process.binding('constants').os;
const { async_id_symbol } = process.binding('async_wrap');
const { nextTick } = require('internal/process/next_tick');
Expand Down Expand Up @@ -450,21 +450,24 @@ Socket.prototype.send = function(buffer,
}

const afterDns = (ex, ip) => {
doSend(ex, this, ip, list, address, port, callback);
defaultTriggerAsyncIdScope(
this[async_id_symbol],
[ex, this, ip, list, address, port, callback],
doSend
);
};

this._handle.lookup(address, afterDns);
};


function doSend(ex, self, ip, list, address, port, callback) {
if (ex) {
if (typeof callback === 'function') {
callback(ex);
process.nextTick(callback, ex);
return;
}

self.emit('error', ex);
process.nextTick(() => self.emit('error', ex));
return;
} else if (!self._handle) {
return;
Expand All @@ -478,20 +481,18 @@ function doSend(ex, self, ip, list, address, port, callback) {
req.callback = callback;
req.oncomplete = afterSend;
}
// 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);

if (err && callback) {
// don't emit as error, dgram_legacy.js compatibility
const ex = exceptionWithHostPort(err, 'send', address, port);
nextTick(self[async_id_symbol], callback, ex);
process.nextTick(callback, ex);
}
}

Expand Down
47 changes: 25 additions & 22 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const async_wrap = process.binding('async_wrap');
* kTriggerAsyncId: The trigger_async_id of the resource responsible for
* the current execution stack.
* kAsyncIdCounter: Incremental counter tracking the next assigned async_id.
* kInitTriggerAsyncId: Written immediately before a resource's constructor
* kDefaultTriggerAsyncId: Written immediately before a resource's constructor
* that sets the value of the init()'s triggerAsyncId. The order of
* retrieving the triggerAsyncId value is passing directly to the
* constructor -> value set in kInitTriggerAsyncId -> executionAsyncId of
* constructor -> value set in kDefaultTriggerAsyncId -> executionAsyncId of
* the current resource.
*/
const { async_hook_fields, async_id_fields } = async_wrap;
Expand Down Expand Up @@ -61,7 +61,7 @@ const active_hooks = {
// for a given step, that step can bail out early.
const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve,
kCheck, kExecutionAsyncId, kAsyncIdCounter,
kInitTriggerAsyncId } = async_wrap.constants;
kDefaultTriggerAsyncId } = async_wrap.constants;

// Used in AsyncHook and AsyncResource.
const init_symbol = Symbol('init');
Expand Down Expand Up @@ -245,25 +245,32 @@ function newUid() {
return ++async_id_fields[kAsyncIdCounter];
}


// 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 triggerAsyncId = async_id_fields[kInitTriggerAsyncId];
// Reset value after it's been called so the next constructor doesn't
// inherit it by accident.
async_id_fields[kInitTriggerAsyncId] = 0;
if (triggerAsyncId <= 0)
triggerAsyncId = async_id_fields[kExecutionAsyncId];
return triggerAsyncId;
function getDefaultTriggerAsyncId() {
var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
// If defaultTriggerAsyncId isn't set, use the executionAsyncId
if (defaultTriggerAsyncId < 0)
defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId];
return defaultTriggerAsyncId;
}


function setInitTriggerId(triggerAsyncId) {
function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) {
// CHECK(Number.isSafeInteger(triggerAsyncId))
// CHECK(triggerAsyncId > 0)
async_id_fields[kInitTriggerAsyncId] = triggerAsyncId;
const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId;

var ret;
try {
ret = Reflect.apply(block, null, opaque);
} finally {
async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId;
}

return ret;
}


Expand All @@ -282,13 +289,9 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
return;

// This can run after the early return check b/c running this function
// manually means that the embedder must have used initTriggerId().
// manually means that the embedder must have used getDefaultTriggerAsyncId().
if (triggerAsyncId === null) {
triggerAsyncId = initTriggerId();
} else {
// If a triggerAsyncId was passed, any kInitTriggerAsyncId still must be
// null'd.
async_id_fields[kInitTriggerAsyncId] = 0;
triggerAsyncId = getDefaultTriggerAsyncId();
}

emitInitNative(asyncId, type, triggerAsyncId, resource);
Expand Down Expand Up @@ -340,8 +343,8 @@ module.exports = {
disableHooks,
// Internal Embedder API
newUid,
initTriggerId,
setInitTriggerId,
getDefaultTriggerAsyncId,
defaultTriggerAsyncIdScope,
emitInit: emitInitScript,
emitBefore: emitBeforeScript,
emitAfter: emitAfterScript,
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,14 @@
// Internal functions needed to manipulate the stack.
const { clearAsyncIdStack, asyncIdStackSize } = async_wrap;
const { kAfter, kExecutionAsyncId,
kInitTriggerAsyncId } = async_wrap.constants;
kDefaultTriggerAsyncId } = async_wrap.constants;

process._fatalException = function(er) {
var caught;

// It's possible that kInitTriggerAsyncId was set for a constructor call
// that threw and was never cleared. So clear it now.
async_id_fields[kInitTriggerAsyncId] = 0;
// It's possible that kDefaultTriggerAsyncId was set for a constructor
// call that threw and was never cleared. So clear it now.
async_id_fields[kDefaultTriggerAsyncId] = -1;

if (exceptionHandlerState.captureFn !== null) {
exceptionHandlerState.captureFn(er);
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/process/next_tick.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function setupNextTick() {
const promises = require('internal/process/promises');
const errors = require('internal/errors');
const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks);
const initTriggerId = async_hooks.initTriggerId;
const getDefaultTriggerAsyncId = async_hooks.getDefaultTriggerAsyncId;
// Two arrays that share state between C++ and JS.
const { async_hook_fields, async_id_fields } = async_wrap;
// Used to change the state of the async id stack.
Expand Down Expand Up @@ -210,7 +210,7 @@ function setupNextTick() {
nextTickQueue.push(new TickObject(callback,
args,
++async_id_fields[kAsyncIdCounter],
initTriggerId()));
getDefaultTriggerAsyncId()));
}

// `internalNextTick()` will not enqueue any callback when the process is
Expand All @@ -237,7 +237,7 @@ function setupNextTick() {
}

if (triggerAsyncId === null)
triggerAsyncId = initTriggerId();
triggerAsyncId = getDefaultTriggerAsyncId();
// In V8 6.2, moving tickInfo & async_id_fields[kAsyncIdCounter] into the
// TickObject incurs a significant performance penalty in the
// next-tick-breadth-args benchmark (revisit later)
Expand Down
91 changes: 46 additions & 45 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const { TCPConnectWrap } = process.binding('tcp_wrap');
const { PipeConnectWrap } = process.binding('pipe_wrap');
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
const { async_id_symbol } = process.binding('async_wrap');
const { newUid, setInitTriggerId } = require('internal/async_hooks');
const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { nextTick } = require('internal/process/next_tick');
const errors = require('internal/errors');
const dns = require('dns');
Expand Down Expand Up @@ -277,6 +277,14 @@ Socket.prototype._unrefTimer = function _unrefTimer() {
timers._unrefActive(s);
};


function shutdownSocket(self, callback) {
var req = new ShutdownWrap();
req.oncomplete = callback;
req.handle = self._handle;
return self._handle.shutdown(req);
}

// the user has called .end(), and all the bytes have been
// sent out to the other side.
function onSocketFinish() {
Expand All @@ -298,14 +306,9 @@ function onSocketFinish() {
if (!this._handle || !this._handle.shutdown)
return this.destroy();

var req = new ShutdownWrap();
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
// trigger id until now.
setInitTriggerId(this[async_id_symbol]);
var err = this._handle.shutdown(req);
var err = defaultTriggerAsyncIdScope(
this[async_id_symbol], [this, afterShutdown], shutdownSocket
);

if (err)
return this.destroy(errnoException(err, 'shutdown'));
Expand Down Expand Up @@ -945,23 +948,15 @@ function internalConnect(
req.localAddress = localAddress;
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);

} 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);
}

Expand Down Expand Up @@ -1030,7 +1025,9 @@ Socket.prototype.connect = function(...args) {
'string',
path);
}
internalConnect(this, path);
defaultTriggerAsyncIdScope(
this[async_id_symbol], [this, path], internalConnect
);
} else {
lookupAndConnect(this, options);
}
Expand Down Expand Up @@ -1073,7 +1070,11 @@ function lookupAndConnect(self, options) {
if (addressType) {
nextTick(self[async_id_symbol], function() {
if (self.connecting)
internalConnect(self, host, port, addressType, localAddress, localPort);
defaultTriggerAsyncIdScope(
self[async_id_symbol],
[self, host, port, addressType, localAddress, localPort],
internalConnect
);
});
return;
}
Expand All @@ -1097,33 +1098,33 @@ 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) {
self.emit('lookup', err, ip, addressType, host);
defaultTriggerAsyncIdScope(self[async_id_symbol], [], function() {
lookup(host, dnsopts, function emitLookup(err, ip, addressType) {
self.emit('lookup', err, ip, addressType, host);

// It's possible we were destroyed while looking this up.
// XXX it would be great if we could cancel the promise returned by
// the look up.
if (!self.connecting) return;
// It's possible we were destroyed while looking this up.
// XXX it would be great if we could cancel the promise returned by
// the look up.
if (!self.connecting) return;

if (err) {
// net.createConnection() creates a net.Socket object and
// immediately calls net.Socket.connect() on it (that's us).
// There are no event listeners registered yet so defer the
// error event to the next tick.
err.host = options.host;
err.port = options.port;
err.message = err.message + ' ' + options.host + ':' + options.port;
process.nextTick(connectErrorNT, self, err);
} else {
self._unrefTimer();
internalConnect(self,
ip,
port,
addressType,
localAddress,
localPort);
}
if (err) {
// net.createConnection() creates a net.Socket object and
// immediately calls net.Socket.connect() on it (that's us).
// There are no event listeners registered yet so defer the
// error event to the next tick.
err.host = options.host;
err.port = options.port;
err.message = err.message + ' ' + options.host + ':' + options.port;
process.nextTick(connectErrorNT, self, err);
} else {
self._unrefTimer();
defaultTriggerAsyncIdScope(
self[async_id_symbol],
[self, ip, port, addressType, localAddress, localPort],
internalConnect
);
}
});
});
}

Expand Down
Loading