Skip to content

async_hooks: fix default nextTick triggerAsyncId #14026

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.

Sign up for GitHub

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

Closed
AndreasMadsen wants to merge 4 commits into from
Closed

async_hooks: fix default nextTick triggerAsyncId #14026

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5aacec
async_hooks: fix default nextTick triggerAsyncId
AndreasMadsen Jul 1, 2017
836cc68
[squash] remove redundant initTriggerId reset
AndreasMadsen Jul 2, 2017
619b98a
[squash] refack suggestions
AndreasMadsen Jul 3, 2017
41b63e6
[squash] remove unused variable
AndreasMadsen Jul 3, 2017
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
Prev Previous commit
Next Next commit
[squash] refack suggestions
  • Loading branch information
@AndreasMadsen
AndreasMadsen committed Jul 3, 2017
commit 619b98ae8892b045ae2fe340b43c33db57c7b88f
7 changes: 4 additions & 3 deletions lib/internal/process/next_tick.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ function setupNextTick() {
if (process._exiting)
return;

if (triggerAsyncId === null) {
triggerAsyncId = async_hooks.initTriggerId();
}

var args;
switch (arguments.length) {
case 2: break;
Expand All @@ -315,9 +319,6 @@ function setupNextTick() {
}

const asyncId = ++async_uid_fields[kAsyncUidCntr];
if (triggerAsyncId === null) {
triggerAsyncId = async_hooks.initTriggerId();
}
const obj = new TickObject(callback, args, asyncId, triggerAsyncId);
nextTickQueue.push(obj);
++tickInfo[kLength];
Expand Down
8 changes: 4 additions & 4 deletions test/async-hooks/init-hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ class ActivityCollector {
this._logid = logid;
this._logtype = logtype;

// register event handlers if provided
// Register event handlers if provided
this.oninit = typeof oninit === 'function' ? oninit : noop;
this.onbefore = typeof onbefore === 'function' ? onbefore : noop;
this.onafter = typeof onafter === 'function' ? onafter : noop;
this.ondestroy = typeof ondestroy === 'function' ? ondestroy : noop;

// create the hook with which we'll collect activity data
// Create the hook with which we'll collect activity data
this._asyncHook = async_hooks.createHook({
init: this._init.bind(this),
before: this._before.bind(this),
Expand Down Expand Up @@ -148,7 +148,7 @@ class ActivityCollector {
_getActivity(uid, hook) {
const h = this._activities.get(uid);
if (!h) {
// if we allowed handles without init we ignore any further life time
// If we allowed handles without init we ignore any further life time
// events this makes sense for a few tests in which we enable some hooks
Copy link
Copy Markdown
Contributor

@refack refack Jul 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: If you're already here, could you turn this comment to a proper paragraph (capitalization, some punctuation, etc)

// later
if (this._allowNoInit) {
Expand All @@ -172,7 +172,7 @@ class ActivityCollector {
uid,
type,
triggerAsyncId,
// in some cases (Timeout) the handle is a function, thus the usual
// In some cases (e.g. Timeout) the handle is a function, thus the usual
// `typeof handle === 'object' && handle !== null` check can't be used.
handleIsObject: handle instanceof Object
};
Expand Down
16 changes: 13 additions & 3 deletions test/async-hooks/test-internal-nexttick-default-trigger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
'use strict';
Copy link
Copy Markdown
Contributor

@refack refack Jul 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: style as in https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure (and but the //Flags on the first line 🤷‍♂️


// Flags: --expose-internals

const common = require('../common');

// This tests ensures that the triggerId of both the internal and external
// nexTick function sets the triggerAsyncId correctly.

const assert = require('assert');
const async_hooks = require('async_hooks');
const initHooks = require('./init-hooks');
Expand All @@ -19,11 +21,16 @@ process.nextTick(common.mustCall(function() {
assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
}));

// internal
// internal default
internal.nextTick(null, common.mustCall(function() {
assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
}));

// internal
internal.nextTick(rootAsyncId + 1, common.mustCall(function() {
assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId + 1);
}));

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

Expand All @@ -34,4 +41,7 @@ process.on('exit', function() {
checkInvocations(as[1], {
init: 1, before: 1, after: 1, destroy: 1
}, 'when process exits');
checkInvocations(as[2], {
init: 1, before: 1, after: 1, destroy: 1
}, 'when process exits');
});