Skip to content

Commit

Permalink
async_hooks: use scope for defaultTriggerAsyncId
Browse files Browse the repository at this point in the history 
Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.

PR-URL: nodejs#17273
Backport-PR-URL: nodejs#18179
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@AndreasMadsen @gibfahn
AndreasMadsen authored and gibfahn committed Jan 17, 2018
1 parent ca3260d commit 52b71a2
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 129 deletions.
8 changes: 0 additions & 8 deletions lib/async_hooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const {
// Sensitive Embedder API
newUid,
getDefaultTriggerAsyncId,
setInitTriggerId,
emitInit,
emitBefore,
emitAfter,
Expand Down Expand Up @@ -281,13 +280,6 @@ Object.defineProperty(module.exports, 'initTriggerId', {
'Use the AsyncResource default instead.', 'DEP0085')
});

Object.defineProperty(module.exports, 'setInitTriggerId', {
get: internalUtil.deprecate(function() {
return setInitTriggerId;
}, 'async_hooks.setInitTriggerId is deprecated. ' +
'Use the triggerAsyncId parameter in AsyncResource instead.', 'DEP0085')
});

Object.defineProperty(module.exports, 'emitInit', {
get: internalUtil.deprecate(function() {
return emitInit;
Expand Down
21 changes: 11 additions & 10 deletions lib/dgram.js
View file
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 { setDefaultTriggerAsyncId } = 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 @@ -448,21 +448,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 @@ -476,20 +479,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.
setDefaultTriggerAsyncId(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
21 changes: 12 additions & 9 deletions lib/internal/async_hooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,20 +250,27 @@ function newUid() {
// constructor is complete.
function getDefaultTriggerAsyncId() {
var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
// Reset value after it's been called so the next constructor doesn't
// inherit it by accident.
async_id_fields[kDefaultTriggerAsyncId] = -1;
// If defaultTriggerAsyncId isn't set, use the executionAsyncId
if (defaultTriggerAsyncId < 0)
defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId];
return defaultTriggerAsyncId;
}


function setDefaultTriggerAsyncId(triggerAsyncId) {
function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) {
// CHECK(Number.isSafeInteger(triggerAsyncId))
// CHECK(triggerAsyncId > 0)
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 @@ -285,10 +292,6 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
// manually means that the embedder must have used getDefaultTriggerAsyncId().
if (triggerAsyncId === null) {
triggerAsyncId = getDefaultTriggerAsyncId();
} else {
// If a triggerAsyncId was passed, any kDefaultTriggerAsyncId still must be
// null'd.
async_id_fields[kDefaultTriggerAsyncId] = -1;
}

emitInitNative(asyncId, type, triggerAsyncId, resource);
Expand Down Expand Up @@ -341,7 +344,7 @@ module.exports = {
// Sensitive Embedder API
newUid,
getDefaultTriggerAsyncId,
setDefaultTriggerAsyncId,
defaultTriggerAsyncIdScope,
emitInit: emitInitScript,
emitBefore: emitBeforeScript,
emitAfter: emitAfterScript,
Expand Down
91 changes: 46 additions & 45 deletions lib/net.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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, setDefaultTriggerAsyncId } = 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 @@ -270,6 +270,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 @@ -291,14 +299,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.
setDefaultTriggerAsyncId(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 @@ -950,23 +953,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.
setDefaultTriggerAsyncId(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.
setDefaultTriggerAsyncId(self[async_id_symbol]);

err = self._handle.connect(req, address, afterConnect);
}

Expand Down Expand Up @@ -1035,7 +1030,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 @@ -1074,7 +1071,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 @@ -1095,33 +1096,33 @@ function lookupAndConnect(self, options) {
debug('connect: dns options', dnsopts);
self._host = host;
var lookup = options.lookup || dns.lookup;
setDefaultTriggerAsyncId(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
11 changes: 6 additions & 5 deletions src/async_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,13 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
if (parent_wrap == nullptr) {
parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true);
}
// get id from parentWrap
double trigger_async_id = parent_wrap->get_async_id();
env->set_default_trigger_async_id(trigger_async_id);
}

wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
env, parent_wrap->get_async_id());
wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
} else {
wrap = PromiseWrap::New(env, promise, nullptr, silent);
}
}

CHECK_NE(wrap, nullptr);
Expand Down
1 change: 0 additions & 1 deletion src/connection_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
};

if (status == 0) {
env->set_default_trigger_async_id(wrap_data->get_async_id());
// Instantiate the client javascript object and handle.
Local<Object> client_obj = WrapType::Instantiate(env,
wrap_data,
Expand Down
33 changes: 16 additions & 17 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,23 +197,27 @@ inline void Environment::AsyncHooks::clear_async_id_stack() {
async_id_fields_[kTriggerAsyncId] = 0;
}

inline Environment::AsyncHooks::InitScope::InitScope(
Environment* env, double init_trigger_async_id)
: env_(env),
async_id_fields_ref_(env->async_hooks()->async_id_fields()) {
if (env_->async_hooks()->fields()[AsyncHooks::kCheck] > 0) {
CHECK_GE(init_trigger_async_id, -1);
inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope
::DefaultTriggerAsyncIdScope(Environment* env,
double default_trigger_async_id)
: async_id_fields_ref_(env->async_hooks()->async_id_fields()) {
if (env->async_hooks()->fields()[AsyncHooks::kCheck] > 0) {
CHECK_GE(default_trigger_async_id, 0);
}
env->async_hooks()->push_async_ids(
async_id_fields_ref_[AsyncHooks::kExecutionAsyncId],
init_trigger_async_id);

old_default_trigger_async_id_ =
async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId];
async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] =
default_trigger_async_id;
}

inline Environment::AsyncHooks::InitScope::~InitScope() {
env_->async_hooks()->pop_async_id(
async_id_fields_ref_[AsyncHooks::kExecutionAsyncId]);
inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope
::~DefaultTriggerAsyncIdScope() {
async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] =
old_default_trigger_async_id_;
}


inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
: env_(env) {
env_->makecallback_cntr_++;
Expand Down Expand Up @@ -473,17 +477,12 @@ inline double Environment::trigger_async_id() {
inline double Environment::get_default_trigger_async_id() {
double default_trigger_async_id =
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId];
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1;
// If defaultTriggerAsyncId isn't set, use the executionAsyncId
if (default_trigger_async_id < 0)
default_trigger_async_id = execution_async_id();
return default_trigger_async_id;
}

inline void Environment::set_default_trigger_async_id(const double id) {
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = id;
}

inline double* Environment::heap_statistics_buffer() const {
CHECK_NE(heap_statistics_buffer_, nullptr);
return heap_statistics_buffer_;
Expand Down
Loading

0 comments on commit 52b71a2

Please sign in to comment.