process: make internal/queue_microtask.js more self-contained

- Instead of passing triggerFatalException through node.js, simply put it on `internalBinding('util')` which has to be loaded anyway. - Expose the implementation of `queueMicrotask` directly instead of through an unnecessary factory function. PR-URL: #25189 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · Jan 17, 2019 · f32e6a8 · f32e6a8
1 parent 2e33ad1
commit f32e6a8
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 41 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -14,8 +14,7 @@
 
 // This file is compiled as if it's wrapped in a function with arguments
 // passed by node::LoadEnvironment()
-/* global process, loaderExports, triggerFatalException */
-/* global isMainThread */
+/* global process, loaderExports, isMainThread */
 
 const { internalBinding, NativeModule } = loaderExports;
 
@@ -619,12 +618,12 @@ function setupGlobalEncoding() {
 
 function setupQueueMicrotask() {
   Object.defineProperty(global, 'queueMicrotask', {
-    get: () => {
+    get() {
       process.emitWarning('queueMicrotask() is experimental.',
                           'ExperimentalWarning');
-      const { setupQueueMicrotask } =
+      const { queueMicrotask } =
         NativeModule.require('internal/queue_microtask');
-      const queueMicrotask = setupQueueMicrotask(triggerFatalException);
+
       Object.defineProperty(global, 'queueMicrotask', {
         value: queueMicrotask,
         writable: true,
@@ -633,7 +632,7 @@ function setupQueueMicrotask() {
       });
       return queueMicrotask;
     },
-    set: (v) => {
+    set(v) {
       Object.defineProperty(global, 'queueMicrotask', {
         value: v,
         writable: true,

diff --git a/lib/internal/queue_microtask.js b/lib/internal/queue_microtask.js
@@ -3,37 +3,36 @@
 const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 const { AsyncResource } = require('async_hooks');
 const { getDefaultTriggerAsyncId } = require('internal/async_hooks');
-const { enqueueMicrotask } = internalBinding('util');
+const {
+  enqueueMicrotask,
+  triggerFatalException
+} = internalBinding('util');
 
-const setupQueueMicrotask = (triggerFatalException) => {
-  const queueMicrotask = (callback) => {
-    if (typeof callback !== 'function') {
-      throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
-    }
+function queueMicrotask(callback) {
+  if (typeof callback !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
+  }
 
-    const asyncResource = new AsyncResource('Microtask', {
-      triggerAsyncId: getDefaultTriggerAsyncId(),
-      requireManualDestroy: true,
-    });
+  const asyncResource = new AsyncResource('Microtask', {
+    triggerAsyncId: getDefaultTriggerAsyncId(),
+    requireManualDestroy: true,
+  });
 
-    enqueueMicrotask(() => {
-      asyncResource.runInAsyncScope(() => {
-        try {
-          callback();
-        } catch (error) {
-          // TODO(devsnek) remove this if
-          // https://bugs.chromium.org/p/v8/issues/detail?id=8326
-          // is resolved such that V8 triggers the fatal exception
-          // handler for microtasks
-          triggerFatalException(error);
-        } finally {
-          asyncResource.emitDestroy();
-        }
-      });
+  enqueueMicrotask(() => {
+    asyncResource.runInAsyncScope(() => {
+      try {
+        callback();
+      } catch (error) {
+        // TODO(devsnek) remove this if
+        // https://bugs.chromium.org/p/v8/issues/detail?id=8326
+        // is resolved such that V8 triggers the fatal exception
+        // handler for microtasks
+        triggerFatalException(error);
+      } finally {
+        asyncResource.emitDestroy();
+      }
     });
-  };
-
-  return queueMicrotask;
-};
+  });
+}
 
-module.exports = { setupQueueMicrotask };
+module.exports = { queueMicrotask };
diff --git a/src/node.cc b/src/node.cc
@@ -1163,18 +1163,14 @@ void LoadEnvironment(Environment* env) {
     return;
   }
 
-  // process, bootstrappers, loaderExports, triggerFatalException
+  // process, loaderExports, isMainThread
   std::vector<Local<String>> node_params = {
       env->process_string(),
       FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
-      FIXED_ONE_BYTE_STRING(isolate, "triggerFatalException"),
       FIXED_ONE_BYTE_STRING(isolate, "isMainThread")};
   std::vector<Local<Value>> node_args = {
       process,
       loader_exports.ToLocalChecked(),
-      env->NewFunctionTemplate(FatalException)
-          ->GetFunction(context)
-          .ToLocalChecked(),
       Boolean::New(isolate, env->is_main_thread())};
 
   if (ExecuteBootstrapper(

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -1,4 +1,5 @@
 #include "node_internals.h"
+#include "node_errors.h"
 #include "node_watchdog.h"
 
 namespace node {
@@ -221,7 +222,7 @@ void Initialize(Local<Object> target,
                              WatchdogHasPendingSigint);
 
   env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
-
+  env->SetMethod(target, "triggerFatalException", FatalException);
   Local<Object> constants = Object::New(env->isolate());
   NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
   NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);