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 <[email protected]>

Author: AndreasMadsen

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;
 varnextTickQueue=newNextTickQueue();
@@ -302,6 +302,10 @@ function setupNextTick(){
 if(process._exiting)
 return;
 
+if(triggerAsyncId===null){
+triggerAsyncId=async_hooks.initTriggerId();
+}
+
 varargs;
 switch(arguments.length){
 case2: 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=typeofoninit==='function' ? oninit : noop;
 this.onbefore=typeofonbefore==='function' ? onbefore : noop;
 this.onafter=typeofonafter==='function' ? onafter : noop;
 this.ondestroy=typeofondestroy==='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){
 consth=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){
-conststub={ uid,type: 'Unknown'};
+conststub={ uid,type: 'Unknown',handleIsObject: true};
 this._activities.set(uid,stub);
 returnstub;
 }else{
@@ -163,7 +168,14 @@ class ActivityCollector{
 }
 
 _init(uid,type,triggerAsyncId,handle){
-constactivity={ uid, type, triggerAsyncId };
+constactivity={
+ 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: handleinstanceofObject
+};
 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
+constcommon=require('../common');
+
+// This tests ensures that the triggerId of both the internal and external
+// nexTick function sets the triggerAsyncId correctly.
+
+constassert=require('assert');
+constasync_hooks=require('async_hooks');
+constinitHooks=require('./init-hooks');
+const{ checkInvocations }=require('./hook-checks');
+constinternal=require('internal/process/next_tick');
+
+consthooks=initHooks();
+hooks.enable();
+
+constrootAsyncId=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();
+
+constas=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');
+});