From 2eabd926396ad956461c0746bfc341420546a41e Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Wed, 5 Jul 2017 14:53:20 +0200
Subject: [PATCH] async_hooks: reduce duplication with factory

PR-URL: https://github.com/nodejs/node/pull/13755
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
---
 lib/async_hooks.js | 100 ++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 64 deletions(-)

diff --git a/lib/async_hooks.js b/lib/async_hooks.js
index e1029c97a57eec..d5c7c116434c0b 100644
--- a/lib/async_hooks.js
+++ b/lib/async_hooks.js
@@ -49,15 +49,18 @@ const init_symbol = Symbol('init');
 const before_symbol = Symbol('before');
 const after_symbol = Symbol('after');
 const destroy_symbol = Symbol('destroy');
+const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative');
+const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative');
+const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative');
 
 // Setup the callbacks that node::AsyncWrap will call when there are hooks to
 // process. They use the same functions as the JS embedder API. These callbacks
 // are setup immediately to prevent async_wrap.setupHooks() from being hijacked
 // and the cost of doing so is negligible.
 async_wrap.setupHooks({ init,
-                        before: emitBeforeN,
-                        after: emitAfterN,
-                        destroy: emitDestroyN });
+                        before: emitBeforeNative,
+                        after: emitAfterNative,
+                        destroy: emitDestroyNative });
 
 // Used to fatally abort the process if a callback throws.
 function fatalError(e) {
@@ -325,8 +328,8 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) {
     triggerAsyncId = initTriggerId();
   }
 
-  // I'd prefer allowing these checks to not exist, or only throw in a debug
-  // build, in order to improve performance.
+  // TODO(trevnorris): I'd prefer allowing these checks to not exist, or only
+  // throw in a debug build, in order to improve performance.
   if (!Number.isSafeInteger(asyncId) || asyncId < 0)
     throw new RangeError('asyncId must be an unsigned integer');
   if (typeof type !== 'string' || type.length <= 0)
@@ -342,24 +345,35 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) {
   }
 }
 
-
-function emitBeforeN(asyncId) {
-  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][before_symbol] === 'function') {
-        active_hooks_array[i][before_symbol](asyncId);
+function emitHookFactory(symbol, name) {
+  // Called from native. The asyncId stack handling is taken care of there
+  // before this is called.
+  // eslint-disable-next-line func-style
+  const fn = function(asyncId) {
+    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][symbol] === 'function') {
+          active_hooks_array[i][symbol](asyncId);
+        }
       }
+    } catch (e) {
+      fatalError(e);
     }
-  } catch (e) {
-    fatalError(e);
-  }
-  processing_hook = false;
+    processing_hook = false;
 
-  if (tmp_active_hooks_array !== null) {
-    restoreTmpHooks();
-  }
+    if (tmp_active_hooks_array !== null) {
+      restoreTmpHooks();
+    }
+  };
+  // Set the name property of the anonymous function as it looks good in the
+  // stack trace.
+  Object.defineProperty(fn, 'name', {
+    value: name
+  });
+  return fn;
 }
 
 
@@ -379,29 +393,7 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) {
 
   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) {
-  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') {
-        active_hooks_array[i][after_symbol](asyncId);
-      }
-    }
-  } catch (e) {
-    fatalError(e);
-  }
-  processing_hook = false;
-
-  if (tmp_active_hooks_array !== null) {
-    restoreTmpHooks();
-  }
+  emitBeforeNative(asyncId);
 }
 
 
@@ -410,7 +402,7 @@ function emitAfterN(asyncId) {
 // after callbacks.
 function emitAfterS(asyncId) {
   if (async_hook_fields[kAfter] > 0)
-    emitAfterN(asyncId);
+    emitAfterNative(asyncId);
 
   popAsyncIds(asyncId);
 }
@@ -425,26 +417,6 @@ function emitDestroyS(asyncId) {
 }
 
 
-function emitDestroyN(asyncId) {
-  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][destroy_symbol] === 'function') {
-        active_hooks_array[i][destroy_symbol](asyncId);
-      }
-    }
-  } catch (e) {
-    fatalError(e);
-  }
-  processing_hook = false;
-
-  if (tmp_active_hooks_array !== null) {
-    restoreTmpHooks();
-  }
-}
-
-
 // Emit callbacks for native calls. Since some state can be setup directly from
 // C++ there's no need to perform all the work here.