async_hooks: use common emitBefore and emitAfter

Timers and nextTick have special emitBefore and emitAfter functions for
historic reasons. These function are not needed any more, thus the
public emitBefore and emitAfter function can be used.

PR-URL: #14050
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: AndreasMadsen

lib/async_hooks.js

@@ -389,9 +389,8 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) {
 
   pushAsyncIds(asyncId, triggerAsyncId);
 
-  if (async_hook_fields[kBefore] === 0)
-    return;
-  emitBeforeNative(asyncId);
+  if (async_hook_fields[kBefore] > 0)
+    emitBeforeNative(asyncId);
 }
 
 

lib/internal/process/next_tick.js

@@ -56,12 +56,9 @@ function setupNextTick() {
   // Two arrays that share state between C++ and JS.
   const { async_hook_fields, async_uid_fields } = async_wrap;
   // Used to change the state of the async id stack.
-  const { pushAsyncIds, popAsyncIds } = async_wrap;
-  // 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 } =
-      async_wrap.constants;
+  const { kInit, kDestroy, kAsyncUidCntr } = async_wrap.constants;
   const { async_id_symbol, trigger_id_symbol } = async_wrap;
   var nextTickQueue = new NextTickQueue();
   var microtasksScheduled = false;
@@ -149,24 +146,6 @@ function setupNextTick() {
     }
   }
 
-  // TODO(trevnorris): Using std::stack of Environment::AsyncHooks::ids_stack_
-  // is much slower here than was the Float64Array stack used in a previous
-  // implementation. Problem is the Float64Array stack was a bit brittle.
-  // Investigate how to harden that implementation and possibly reintroduce it.
-  function nextTickEmitBefore(asyncId, triggerAsyncId) {
-    if (async_hook_fields[kBefore] > 0)
-      emitBefore(asyncId, triggerAsyncId);
-    else
-      pushAsyncIds(asyncId, triggerAsyncId);
-  }
-
-  function nextTickEmitAfter(asyncId) {
-    if (async_hook_fields[kAfter] > 0)
-      emitAfter(asyncId);
-    else
-      popAsyncIds(asyncId);
-  }
-
   // Run callbacks that have no domain.
   // Using domains will cause this to be overridden.
   function _tickCallback() {
@@ -182,7 +161,7 @@ function setupNextTick() {
         // CHECK(Number.isSafeInteger(tock[trigger_id_symbol]))
         // CHECK(tock[trigger_id_symbol] > 0)
 
-        nextTickEmitBefore(tock[async_id_symbol], tock[trigger_id_symbol]);
+        emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]);
         // emitDestroy() places the async_id_symbol into an asynchronous queue
         // that calls the destroy callback in the future. It's called before
         // calling tock.callback so destroy will be called even if the callback
@@ -200,7 +179,7 @@ function setupNextTick() {
         // performance hit associated with using `fn.apply()`
         _combinedTickCallback(args, callback);
 
-        nextTickEmitAfter(tock[async_id_symbol]);
+        emitAfter(tock[async_id_symbol]);
 
         if (kMaxCallbacksPerLoop < tickInfo[kIndex])
           tickDone();
@@ -227,7 +206,7 @@ function setupNextTick() {
         // CHECK(Number.isSafeInteger(tock[trigger_id_symbol]))
         // CHECK(tock[trigger_id_symbol] > 0)
 
-        nextTickEmitBefore(tock[async_id_symbol], tock[trigger_id_symbol]);
+        emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]);
         // TODO(trevnorris): See comment in _tickCallback() as to why this
         // isn't a good solution.
         if (async_hook_fields[kDestroy] > 0)
@@ -238,7 +217,7 @@ function setupNextTick() {
         // performance hit associated with using `fn.apply()`
         _combinedTickCallback(args, callback);
 
-        nextTickEmitAfter(tock[async_id_symbol]);
+        emitAfter(tock[async_id_symbol]);
 
         if (kMaxCallbacksPerLoop < tickInfo[kIndex])
           tickDone();

lib/timers.js

@@ -34,13 +34,10 @@ const kOnTimeout = TimerWrap.kOnTimeout | 0;
 const initTriggerId = async_hooks.initTriggerId;
 // Two arrays that share state between C++ and JS.
 const { async_hook_fields, async_uid_fields } = async_wrap;
-// Used to change the state of the async id stack.
-const { pushAsyncIds, popAsyncIds } = async_wrap;
 // 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 } =
-    async_wrap.constants;
+const { kInit, kDestroy, kAsyncUidCntr } = async_wrap.constants;
 // Symbols for storing async id state.
 const async_id_symbol = Symbol('asyncId');
 const trigger_id_symbol = Symbol('triggerAsyncId');
@@ -149,22 +146,6 @@ exports._unrefActive = function(item) {
 };
 
 
-function timerEmitBefore(asyncId, triggerAsyncId) {
-  if (async_hook_fields[kBefore] > 0)
-    emitBefore(asyncId, triggerAsyncId);
-  else
-    pushAsyncIds(asyncId, triggerAsyncId);
-}
-
-
-function timerEmitAfter(asyncId) {
-  if (async_hook_fields[kAfter] > 0)
-    emitAfter(asyncId);
-  else
-    popAsyncIds(asyncId);
-}
-
-
 // The underlying logic for scheduling or re-scheduling a timer.
 //
 // Appends a timer onto the end of an existing timers list, or creates a new
@@ -318,14 +299,14 @@ function tryOnTimeout(timer, list) {
       timer[async_id_symbol] : null;
   var threw = true;
   if (timerAsyncId !== null)
-    timerEmitBefore(timerAsyncId, timer[trigger_id_symbol]);
+    emitBefore(timerAsyncId, timer[trigger_id_symbol]);
   try {
     ontimeout(timer);
     threw = false;
   } finally {
     if (timerAsyncId !== null) {
       if (!threw)
-        timerEmitAfter(timerAsyncId);
+        emitAfter(timerAsyncId);
       if (!timer._repeat && async_hook_fields[kDestroy] > 0 &&
           !timer._destroyed) {
         emitDestroy(timerAsyncId);
@@ -756,7 +737,7 @@ function processImmediate() {
 // 4.7) what is in this smaller function.
 function tryOnImmediate(immediate, oldTail) {
   var threw = true;
-  timerEmitBefore(immediate[async_id_symbol], immediate[trigger_id_symbol]);
+  emitBefore(immediate[async_id_symbol], immediate[trigger_id_symbol]);
   try {
     // make the actual call outside the try/catch to allow it to be optimized
     runCallback(immediate);
@@ -765,7 +746,7 @@ function tryOnImmediate(immediate, oldTail) {
     // clearImmediate checks _callback === null for kDestroy hooks.
     immediate._callback = null;
     if (!threw)
-      timerEmitAfter(immediate[async_id_symbol]);
+      emitAfter(immediate[async_id_symbol]);
     if (async_hook_fields[kDestroy] > 0 && !immediate._destroyed) {
       emitDestroy(immediate[async_id_symbol]);
       immediate._destroyed = true;

test/async-hooks/test-callback-error.js

@@ -12,32 +12,33 @@ switch (arg) {
       oninit: common.mustCall(() => { throw new Error(arg); })
     }).enable();
     async_hooks.emitInit(
-      async_hooks.executionAsyncId(),
+      async_hooks.newUid(),
       `${arg}_type`,
-      async_hooks.triggerAsyncId()
+      async_hooks.executionAsyncId()
     );
     return;
 
   case 'test_callback':
     initHooks({
       onbefore: common.mustCall(() => { throw new Error(arg); })
     }).enable();
+    const newAsyncId = async_hooks.newUid();
     async_hooks.emitInit(
-      async_hooks.executionAsyncId(),
+      newAsyncId,
       `${arg}_type`,
-      async_hooks.triggerAsyncId()
+      async_hooks.executionAsyncId()
     );
-    async_hooks.emitBefore(async_hooks.executionAsyncId());
+    async_hooks.emitBefore(newAsyncId, async_hooks.executionAsyncId());
     return;
 
   case 'test_callback_abort':
     initHooks({
       oninit: common.mustCall(() => { throw new Error(arg); })
     }).enable();
     async_hooks.emitInit(
-      async_hooks.executionAsyncId(),
+      async_hooks.newUid(),
       `${arg}_type`,
-      async_hooks.triggerAsyncId()
+      async_hooks.executionAsyncId()
     );
     return;
 }

test/async-hooks/test-emit-before-after.js

@@ -8,17 +8,19 @@ const initHooks = require('./init-hooks');
 
 switch (process.argv[2]) {
   case 'test_invalid_async_id':
-    async_hooks.emitBefore(-1);
-    break;
+    async_hooks.emitBefore(-1, 1);
+    return;
   case 'test_invalid_trigger_id':
     async_hooks.emitBefore(1, -1);
-    break;
+    return;
 }
+assert.ok(!process.argv[2]);
+
 
 const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']);
 assert.strictEqual(c1.stderr.toString().split(/[\r\n]+/g)[0],
                    'Error: before(): asyncId or triggerAsyncId is less than ' +
-                   'zero (asyncId: -1, triggerAsyncId: -1)');
+                   'zero (asyncId: -1, triggerAsyncId: 1)');
 assert.strictEqual(c1.status, 1);
 
 const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']);
@@ -32,7 +34,7 @@ const expectedTriggerId = async_hooks.newUid();
 const expectedType = 'test_emit_before_after_type';
 
 // Verify that if there is no registered hook, then nothing will happen.
-async_hooks.emitBefore(expectedId);
+async_hooks.emitBefore(expectedId, expectedTriggerId);
 async_hooks.emitAfter(expectedId);
 
 initHooks({
@@ -42,5 +44,5 @@ initHooks({
 }).enable();
 
 async_hooks.emitInit(expectedId, expectedType, expectedTriggerId);
-async_hooks.emitBefore(expectedId);
+async_hooks.emitBefore(expectedId, expectedTriggerId);
 async_hooks.emitAfter(expectedId);