async_hooks: minor refactor to callback invocation

Re-use the `init` function wherever possible, and move
`try { … } catch` blocks that result in fatal errors to a larger
scope.

Also make the argument order for `init()` consistent in the codebase.

PR-URL: https://github.com/nodejs/node/pull/13419
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

lib/async_hooks.js

@@ -214,17 +214,7 @@ class AsyncResource {
     if (async_hook_fields[kInit] === 0)
       return;
 
-    processing_hook = true;
+    init(this[async_id_symbol], type, triggerId, this);
-    for (var i = 0; i < active_hooks_array.length; i++) {
-      if (typeof active_hooks_array[i][init_symbol] === 'function') {
-        runInitCallback(active_hooks_array[i][init_symbol],
-                        this[async_id_symbol],
-                        type,
-                        triggerId,
-                        this);
-      }
-    }
-    processing_hook = false;
   }
 
   emitBefore() {
@@ -321,14 +311,7 @@ function emitInitS(asyncId, type, triggerId, resource) {
   if (!Number.isSafeInteger(triggerId) || triggerId < 0)
     throw new RangeError('triggerId must be an unsigned integer');
 
-  processing_hook = true;
+  init(asyncId, type, triggerId, resource);
-  for (var i = 0; i < active_hooks_array.length; i++) {
-    if (typeof active_hooks_array[i][init_symbol] === 'function') {
-      runInitCallback(
-        active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource);
-    }
-  }
-  processing_hook = false;
 
   // Isn't null if hooks were added/removed while the hooks were running.
   if (tmp_active_hooks_array !== null) {
@@ -339,10 +322,15 @@ function emitInitS(asyncId, type, triggerId, resource) {
 
 function emitBeforeN(asyncId) {
   processing_hook = true;
-  for (var i = 0; i < active_hooks_array.length; i++) {
+  // Use a single try/catch for all hook to avoid setting up one per iteration.
-    if (typeof active_hooks_array[i][before_symbol] === 'function') {
+  try {
-      runCallback(active_hooks_array[i][before_symbol], asyncId);
+    for (var i = 0; i < active_hooks_array.length; i++) {
+      if (typeof active_hooks_array[i][before_symbol] === 'function') {
+        active_hooks_array[i][before_symbol](asyncId);
+      }
     }
+  } catch (e) {
+    fatalError(e);
   }
   processing_hook = false;
 
@@ -366,10 +354,8 @@ function emitBeforeS(asyncId, triggerId = asyncId) {
 
   pushAsyncIds(asyncId, triggerId);
 
-  if (async_hook_fields[kBefore] === 0) {
+  if (async_hook_fields[kBefore] === 0)
     return;
-  }
-
   emitBeforeN(asyncId);
 }
 
@@ -377,15 +363,18 @@ function emitBeforeS(asyncId, triggerId = asyncId) {
 // Called from native. The asyncId stack handling is taken care of there before
 // this is called.
 function emitAfterN(asyncId) {
-  if (async_hook_fields[kAfter] > 0) {
+  processing_hook = true;
-    processing_hook = true;
+  // Use a single try/catch for all hook to avoid setting up one per iteration.
+  try {
     for (var i = 0; i < active_hooks_array.length; i++) {
       if (typeof active_hooks_array[i][after_symbol] === 'function') {
-        runCallback(active_hooks_array[i][after_symbol], asyncId);
+        active_hooks_array[i][after_symbol](asyncId);
       }
     }
-    processing_hook = false;
+  } catch (e) {
+    fatalError(e);
   }
+  processing_hook = false;
 
   if (tmp_active_hooks_array !== null) {
     restoreTmpHooks();
@@ -397,7 +386,9 @@ function emitAfterN(asyncId) {
 // kIdStackIndex. But what happens if the user doesn't have both before and
 // after callbacks.
 function emitAfterS(asyncId) {
-  emitAfterN(asyncId);
+  if (async_hook_fields[kAfter] > 0)
+    emitAfterN(asyncId);
+
   popAsyncIds(asyncId);
 }
 
@@ -413,10 +404,15 @@ function emitDestroyS(asyncId) {
 
 function emitDestroyN(asyncId) {
   processing_hook = true;
-  for (var i = 0; i < active_hooks_array.length; i++) {
+  // Use a single try/catch for all hook to avoid setting up one per iteration.
-    if (typeof active_hooks_array[i][destroy_symbol] === 'function') {
+  try {
-      runCallback(active_hooks_array[i][destroy_symbol], asyncId);
+    for (var i = 0; i < active_hooks_array.length; i++) {
+      if (typeof active_hooks_array[i][destroy_symbol] === 'function') {
+        active_hooks_array[i][destroy_symbol](asyncId);
+      }
     }
+  } catch (e) {
+    fatalError(e);
   }
   processing_hook = false;
 
@@ -434,44 +430,27 @@ function emitDestroyN(asyncId) {
 // init().
 // TODO(trevnorris): Perhaps have MakeCallback call a single JS function that
 // does the before/callback/after calls to remove two additional calls to JS.
-function init(asyncId, type, resource, triggerId) {
+
+// Force the application to shutdown if one of the callbacks throws. This may
+// change in the future depending on whether it can be determined if there's a
+// slim chance of the application remaining stable after handling one of these
+// exceptions.
+function init(asyncId, type, triggerId, resource) {
   processing_hook = true;
-  for (var i = 0; i < active_hooks_array.length; i++) {
+  // Use a single try/catch for all hook to avoid setting up one per iteration.
-    if (typeof active_hooks_array[i][init_symbol] === 'function') {
+  try {
-      runInitCallback(
+    for (var i = 0; i < active_hooks_array.length; i++) {
-        active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource);
+      if (typeof active_hooks_array[i][init_symbol] === 'function') {
+        active_hooks_array[i][init_symbol](asyncId, type, triggerId, resource);
+      }
     }
+  } catch (e) {
+    fatalError(e);
   }
   processing_hook = false;
 }
 
 
-// Generalized callers for all callbacks that handles error handling.
-
-// If either runInitCallback() or runCallback() throw then force the
-// application to shutdown if one of the callbacks throws. This may change in
-// the future depending on whether it can be determined if there's a slim
-// chance of the application remaining stable after handling one of these
-// exceptions.
-
-function runInitCallback(cb, asyncId, type, triggerId, resource) {
-  try {
-    cb(asyncId, type, triggerId, resource);
-  } catch (e) {
-    fatalError(e);
-  }
-}
-
-
-function runCallback(cb, asyncId) {
-  try {
-    cb(asyncId);
-  } catch (e) {
-    fatalError(e);
-  }
-}
-
-
 // Placing all exports down here because the exported classes won't export
 // otherwise.
 module.exports = {

src/async-wrap.cc

@@ -560,8 +560,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
   Local<Value> argv[] = {
     Number::New(env->isolate(), async_id),
     type,
-    object,
     Number::New(env->isolate(), trigger_id),
+    object,
   };
 
   TryCatch try_catch(env->isolate());