Skip to content

Commit

Permalink
async_hooks,process: remove internalNextTick
Browse files Browse the repository at this point in the history
Instead of having mostly duplicate code in form of internalNextTick,
instead use the existing defaultAsyncTriggerIdScope with a slight
modification which allows undefined triggerAsyncId to be passed in,
which then just triggers the callback with the provided arguments.

PR-URL: nodejs#19147
Refs: nodejs#19104
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
apapirovski authored and MayaLekova committed May 8, 2018
1 parent 1a22b77 commit 350cb8d
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 119 deletions.
10 changes: 5 additions & 5 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const util = require('util');
const EventEmitter = require('events');
const debug = util.debuglog('http');
const { async_id_symbol } = require('internal/async_hooks').symbols;
const { nextTick } = require('internal/process/next_tick');

// New Agent code.

Expand Down Expand Up @@ -342,10 +341,7 @@ Agent.prototype.destroy = function destroy() {
function handleSocketCreation(request, informRequest) {
return function handleSocketCreation_Inner(err, socket) {
if (err) {
const asyncId = (socket && socket._handle && socket._handle.getAsyncId) ?
socket._handle.getAsyncId() :
null;
nextTick(asyncId, () => request.emit('error', err));
process.nextTick(emitErrorNT, request, err);
return;
}
if (informRequest)
Expand All @@ -355,6 +351,10 @@ function handleSocketCreation(request, informRequest) {
};
}

function emitErrorNT(emitter, err) {
emitter.emit('error', err);
}

module.exports = {
Agent,
globalAgent: new Agent()
Expand Down
6 changes: 3 additions & 3 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ const {
const { OutgoingMessage } = require('_http_outgoing');
const Agent = require('_http_agent');
const { Buffer } = require('buffer');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { urlToOptions, searchParamsSymbol } = require('internal/url');
const { outHeadersKey, ondrain } = require('internal/http');
const { nextTick } = require('internal/process/next_tick');
const {
ERR_HTTP_HEADERS_SENT,
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -563,10 +563,10 @@ function responseKeepAlive(res, req) {
socket.once('error', freeSocketErrorListener);
// There are cases where _handle === null. Avoid those. Passing null to
// nextTick() will call getDefaultTriggerAsyncId() to retrieve the id.
const asyncId = socket._handle ? socket._handle.getAsyncId() : null;
const asyncId = socket._handle ? socket._handle.getAsyncId() : undefined;
// Mark this socket as available, AFTER user-added end
// handlers have a chance to run.
nextTick(asyncId, emitFreeNT, socket);
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, socket);
}
}

Expand Down
28 changes: 16 additions & 12 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ const common = require('_http_common');
const checkIsHttpToken = common._checkIsHttpToken;
const checkInvalidHeaderChar = common._checkInvalidHeaderChar;
const { outHeadersKey } = require('internal/http');
const { async_id_symbol } = require('internal/async_hooks').symbols;
const { nextTick } = require('internal/process/next_tick');
const {
defaultTriggerAsyncIdScope,
symbols: { async_id_symbol }
} = require('internal/async_hooks');
const {
ERR_HTTP_HEADERS_SENT,
ERR_HTTP_INVALID_HEADER_VALUE,
Expand Down Expand Up @@ -272,15 +274,14 @@ function _writeRaw(data, encoding, callback) {
this._flushOutput(conn);
} else if (!data.length) {
if (typeof callback === 'function') {
let socketAsyncId = this.socket[async_id_symbol];
// If the socket was set directly it won't be correctly initialized
// with an async_id_symbol.
// TODO(AndreasMadsen): @trevnorris suggested some more correct
// solutions in:
// https://github.com/nodejs/node/pull/14389/files#r128522202
if (socketAsyncId === undefined) socketAsyncId = null;

nextTick(socketAsyncId, callback);
defaultTriggerAsyncIdScope(conn[async_id_symbol],
process.nextTick,
callback);
}
return true;
}
Expand Down Expand Up @@ -633,10 +634,13 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
function write_(msg, chunk, encoding, callback, fromEnd) {
if (msg.finished) {
const err = new ERR_STREAM_WRITE_AFTER_END();
nextTick(msg.socket && msg.socket[async_id_symbol],
writeAfterEndNT.bind(msg),
err,
callback);
const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
defaultTriggerAsyncIdScope(triggerAsyncId,
process.nextTick,
writeAfterEndNT,
msg,
err,
callback);

return true;
}
Expand Down Expand Up @@ -686,8 +690,8 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
}


function writeAfterEndNT(err, callback) {
this.emit('error', err);
function writeAfterEndNT(msg, err, callback) {
msg.emit('error', err);
if (callback) callback(err);
}

Expand Down
6 changes: 4 additions & 2 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const {
symbols: { async_id_symbol }
} = require('internal/async_hooks');
const { UV_UDP_REUSEADDR } = process.binding('constants').os;
const { nextTick } = require('internal/process/next_tick');

const { UDP, SendWrap } = process.binding('udp_wrap');

Expand Down Expand Up @@ -526,7 +525,10 @@ Socket.prototype.close = function(callback) {
this._stopReceiving();
this._handle.close();
this._handle = null;
nextTick(this[async_id_symbol], socketCloseNT, this);
defaultTriggerAsyncIdScope(this[async_id_symbol],
process.nextTick,
socketCloseNT,
this);

return this;
};
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ function clearDefaultTriggerAsyncId() {


function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
if (triggerAsyncId === undefined)
return Reflect.apply(block, null, args);
// CHECK(Number.isSafeInteger(triggerAsyncId))
// CHECK(triggerAsyncId > 0)
const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
Expand Down
33 changes: 0 additions & 33 deletions lib/internal/process/next_tick.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
'use strict';

exports.setup = setupNextTick;
// Will be overwritten when setupNextTick() is called.
exports.nextTick = null;

function setupNextTick() {
const {
Expand Down Expand Up @@ -87,9 +85,6 @@ function setupNextTick() {
// Needs to be accessible from beyond this scope.
process._tickCallback = _tickCallback;

// Set the nextTick() function for internal usage.
exports.nextTick = internalNextTick;

function _tickCallback() {
let tock;
do {
Expand Down Expand Up @@ -165,32 +160,4 @@ function setupNextTick() {

push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
}

// `internalNextTick()` will not enqueue any callback when the process is
// about to exit since the callback would not have a chance to be executed.
function internalNextTick(triggerAsyncId, callback) {
if (typeof callback !== 'function')
throw new ERR_INVALID_CALLBACK();
// CHECK(Number.isSafeInteger(triggerAsyncId) || triggerAsyncId === null)
// CHECK(triggerAsyncId > 0 || triggerAsyncId === null)

if (process._exiting)
return;

var args;
switch (arguments.length) {
case 2: break;
case 3: args = [arguments[2]]; break;
case 4: args = [arguments[2], arguments[3]]; break;
case 5: args = [arguments[2], arguments[3], arguments[4]]; break;
default:
args = new Array(arguments.length - 2);
for (var i = 2; i < arguments.length; i++)
args[i - 2] = arguments[i];
}

if (triggerAsyncId === null)
triggerAsyncId = getDefaultTriggerAsyncId();
push(new TickObject(callback, args, triggerAsyncId));
}
}
29 changes: 20 additions & 9 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const {
defaultTriggerAsyncIdScope,
symbols: { async_id_symbol }
} = require('internal/async_hooks');
const { nextTick } = require('internal/process/next_tick');
const errors = require('internal/errors');
const {
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -392,7 +391,7 @@ function writeAfterFIN(chunk, encoding, cb) {
// TODO: defer error events consistently everywhere, not just the cb
this.emit('error', er);
if (typeof cb === 'function') {
nextTick(this[async_id_symbol], cb, er);
defaultTriggerAsyncIdScope(this[async_id_symbol], process.nextTick, cb, er);
}
}

Expand Down Expand Up @@ -1069,7 +1068,7 @@ function lookupAndConnect(self, options) {
// If host is an IP, skip performing a lookup
var addressType = isIP(host);
if (addressType) {
nextTick(self[async_id_symbol], function() {
defaultTriggerAsyncIdScope(self[async_id_symbol], process.nextTick, () => {
if (self.connecting)
defaultTriggerAsyncIdScope(
self[async_id_symbol],
Expand Down Expand Up @@ -1372,7 +1371,11 @@ function setupListenHandle(address, port, addressType, backlog, fd) {
var ex = exceptionWithHostPort(err, 'listen', address, port);
this._handle.close();
this._handle = null;
nextTick(this[async_id_symbol], emitErrorNT, this, ex);
defaultTriggerAsyncIdScope(this[async_id_symbol],
process.nextTick,
emitErrorNT,
this,
ex);
return;
}

Expand All @@ -1383,7 +1386,10 @@ function setupListenHandle(address, port, addressType, backlog, fd) {
if (this._unref)
this.unref();

nextTick(this[async_id_symbol], emitListeningNT, this);
defaultTriggerAsyncIdScope(this[async_id_symbol],
process.nextTick,
emitListeningNT,
this);
}

Server.prototype._listen2 = setupListenHandle; // legacy alias
Expand Down Expand Up @@ -1590,8 +1596,11 @@ Server.prototype.getConnections = function(cb) {
const self = this;

function end(err, connections) {
const asyncId = self._handle ? self[async_id_symbol] : null;
nextTick(asyncId, cb, err, connections);
defaultTriggerAsyncIdScope(self[async_id_symbol],
process.nextTick,
cb,
err,
connections);
}

if (!this._usingWorkers) {
Expand Down Expand Up @@ -1669,8 +1678,10 @@ Server.prototype._emitCloseIfDrained = function() {
return;
}

const asyncId = this._handle ? this[async_id_symbol] : null;
nextTick(asyncId, emitCloseNT, this);
defaultTriggerAsyncIdScope(this[async_id_symbol],
process.nextTick,
emitCloseNT,
this);
};


Expand Down
47 changes: 0 additions & 47 deletions test/async-hooks/test-internal-nexttick-default-trigger.js

This file was deleted.

28 changes: 28 additions & 0 deletions test/async-hooks/test-nexttick-default-trigger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';
const common = require('../common');

// This tests ensures that the triggerId of the nextTick function sets the
// triggerAsyncId correctly.

const assert = require('assert');
const async_hooks = require('async_hooks');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const hooks = initHooks();
hooks.enable();

const rootAsyncId = async_hooks.executionAsyncId();

process.nextTick(common.mustCall(function() {
assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
}));

process.on('exit', function() {
hooks.sanityCheck();

const as = hooks.activitiesOfTypes('TickObject');
checkInvocations(as[0], {
init: 1, before: 1, after: 1, destroy: 1
}, 'when process exits');
});
9 changes: 1 addition & 8 deletions test/async-hooks/test-no-assert-when-disabled.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
'use strict';
// Flags: --no-force-async-hooks-checks --expose-internals
const common = require('../common');
require('../common');

const async_hooks = require('internal/async_hooks');
const internal = require('internal/process/next_tick');

// Using undefined as the triggerAsyncId.
// Ref: https://github.com/nodejs/node/issues/14386
// Ref: https://github.com/nodejs/node/issues/14381
// Ref: https://github.com/nodejs/node/issues/14368
internal.nextTick(undefined, common.mustCall());

// Negative asyncIds and invalid type name
async_hooks.emitInit(-1, null, -1, {});
Expand Down

0 comments on commit 350cb8d

Please sign in to comment.