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: #13419 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Andreas Madsen <amwebdk@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Author: addaleax

lib/async_hooks.js

@@ -214,17 +214,7 @@ class AsyncResource {
  if (async_hook_fields[kInit] === 0)
  return;
 
- processing_hook = true;
- 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;
+ init(this[async_id_symbol], type, triggerId, this);
  }
 
  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;
- 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;
+ init(asyncId, type, triggerId, resource);
 
  // 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++) {
- if (typeof active_hooks_array[i][before_symbol] === 'function') {
- runCallback(active_hooks_array[i][before_symbol], asyncId);
+ // 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][before_symbol] === 'function') {
+ active_hooks_array[i][before_symbol](asyncId);
+ }
  }
+ } catch (e) {
+ fatalError(e);
  }
  processing_hook = false;
 
@@ -366,26 +354,27 @@ function emitBeforeS(asyncId, triggerId = asyncId) {
 
  pushAsyncIds(asyncId, triggerId);
 
- if (async_hook_fields[kBefore] === 0) {
+ if (async_hook_fields[kBefore] === 0)
  return;
- }
-
  emitBeforeN(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++) {
- if (typeof active_hooks_array[i][destroy_symbol] === 'function') {
- runCallback(active_hooks_array[i][destroy_symbol], asyncId);
+ // 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][destroy_symbol] === 'function') {
+ active_hooks_array[i][destroy_symbol](asyncId);
+ }
  }
+ } catch (e) {
+ fatalError(e);
  }
  processing_hook = false;
 
@@ -434,41 +430,24 @@ 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) {
- processing_hook = true;
- 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;
-}
-
-
-// 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
+// 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) {
+function init(asyncId, type, triggerId, resource) {
+ processing_hook = true;
+ // Use a single try/catch for all hook to avoid setting up one per iteration.
  try {
- cb(asyncId);
+ for (var i = 0; i < active_hooks_array.length; i++) {
+ 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;
 }
 
 

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());