async_hooks: fix default nextTick triggerAsyncId

In the case where triggerAsyncId is null it should default to the
current executionAsyncId. This worked but as a side-effect the resource
object was changed too.

This fix also makes the null check more strict. EmitInitS is not a
documented API, thus there is no reason to be flexible in its input.

Ref: #13548 (comment)
PR-URL: #14026
Reviewed-By: Refael Ackermann <refack@gmail.com>

lib/async_hooks.js

@@ -322,9 +322,7 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) {
 
  // This can run after the early return check b/c running this function
  // manually means that the embedder must have used initTriggerId().
- if (!Number.isSafeInteger(triggerAsyncId)) {
- if (triggerAsyncId !== undefined)
- resource = triggerAsyncId;
+ if (triggerAsyncId === null) {
  triggerAsyncId = initTriggerId();
  }
 

lib/internal/process/next_tick.js

@@ -60,7 +60,7 @@ function setupNextTick() {
  // The needed emit*() functions.
  const { emitInit, emitBefore, emitAfter, emitDestroy } = async_hooks;
  // Grab the constants necessary for working with internal arrays.
- const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr, kInitTriggerId } =
+ const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr } =
  async_wrap.constants;
  const { async_id_symbol, trigger_id_symbol } = async_wrap;
  var nextTickQueue = new NextTickQueue();
@@ -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;
@@ -320,8 +324,5 @@ function setupNextTick() {
  ++tickInfo[kLength];
  if (async_hook_fields[kInit] > 0)
  emitInit(asyncId, 'TickObject', triggerAsyncId, obj);
-
- // The call to initTriggerId() was skipped, so clear kInitTriggerId.
- async_uid_fields[kInitTriggerId] = 0;
  }
 }

test/async-hooks/init-hooks.js

@@ -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),
@@ -106,10 +106,15 @@ class ActivityCollector {
  '\nExpected "destroy" to be called after "after"');
  }
  }
+ if (!a.handleIsObject) {
+ v('No resource object\n' + activityString(a) +
+ '\nExpected "init" to be called with a resource object');
+ }
  }
  if (violations.length) {
- console.error(violations.join('\n'));
- assert.fail(violations.length, 0, `Failed sanity checks: ${violations}`);
+ console.error(violations.join('\n\n') + '\n');
+ assert.fail(violations.length, 0,
+ `${violations.length} failed sanity checks`);
  }
  }
 
@@ -143,11 +148,11 @@ 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
  // later
  if (this._allowNoInit) {
- const stub = { uid, type: 'Unknown' };
+ const stub = { uid, type: 'Unknown', handleIsObject: true };
  this._activities.set(uid, stub);
  return stub;
  } else {
@@ -163,7 +168,14 @@ class ActivityCollector {
  }
 
  _init(uid, type, triggerAsyncId, handle) {
- const activity = { uid, type, triggerAsyncId };
+ const activity = {
+ uid,
+ type,
+ triggerAsyncId,
+ // 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
+ };
  this._stamp(activity, 'init');
  this._activities.set(uid, activity);
  this._maybeLog(uid, type, 'init');

test/async-hooks/test-emit-init.js

@@ -46,4 +46,4 @@ initHooks({
  })
 }).enable();
 
-async_hooks.emitInit(expectedId, expectedType, expectedResource);
+async_hooks.emitInit(expectedId, expectedType, null, expectedResource);

test/async-hooks/test-internal-nexttick-default-trigger.js

@@ -0,0 +1,47 @@
+'use strict';
+// 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');
+const { checkInvocations } = require('./hook-checks');
+const internal = require('internal/process/next_tick');
+
+const hooks = initHooks();
+hooks.enable();
+
+const rootAsyncId = async_hooks.executionAsyncId();
+
+// public
+process.nextTick(common.mustCall(function() {
+ assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
+}));
+
+// 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();
+
+ const as = hooks.activitiesOfTypes('TickObject');
+ checkInvocations(as[0], {
+ init: 1, before: 1, after: 1, destroy: 1
+ }, 'when process exits');
+ 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');
+});