From 8a60be4f0fd6a8f6591b7d591e20960fc4372fe4 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Sun, 23 Dec 2018 09:27:35 +0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/bootstrap/node.js  | 11 +++----
 lib/internal/queue_microtask.js | 57 ++++++++++++++++-----------------
 src/node.cc                     |  6 +---
 src/node_util.cc                |  3 +-
 4 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 5bf2dc523f6fd1..3c14d4851af308 100644
--- 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;
 
@@ -605,12 +604,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,
@@ -619,7 +618,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
index 6421e7f9400e55..6ca0c1e144a92b 100644
--- 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
index 91190788b2a35a..bc359903be1b7a 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1200,18 +1200,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
index f7412d92bcd8e7..2bc210677567a3 100644
--- 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);