process: move eval and exception bootstrap ito process/execution.js

This patch:

- Moves `tryGetCwd`, `evalScript` and `fatalException` from
  `bootstrap/node.js` into `process/execution.js` so that
  they do have to be passed into the worker thread
  setup function, instead the worker code can require them
  when necessary.
- Moves `setUncaughtExceptionCaptureCallback` and
  `hasUncaughtExceptionCaptureCallback` along with the two
  global state `exceptionHandlerState` and
  `shouldAbortOnUncaughtToggle` info `process.execution.js`
  as those are only used by the fatalException and these
  two accessors as one self-contained unit.

PR-URL: https://github.com/nodejs/node/pull/25199
Reviewed-By: James M Snell <jasnell@gmail.com>

6 files changed, 181 insertions, 147 deletions

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index c709635a75..4de951ca54 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -18,7 +18,6 @@
 
 const { internalBinding, NativeModule } = loaderExports;
 
-const exceptionHandlerState = { captureFn: null };
 let getOptionValue;
 
 function startup() {
@@ -26,8 +25,23 @@ function startup() {
 
   setupProcessObject();
 
-  // Do this good and early, since it handles errors.
-  setupProcessFatal();
+  // TODO(joyeecheung): this does not have to done so early, any fatal errors
+  // thrown before user code execution should simply crash the process
+  // and we do not care about any clean up at that point. We don't care
+  // about emitting any events if the process crash upon bootstrap either.
+  {
+    const {
+      fatalException,
+      setUncaughtExceptionCaptureCallback,
+      hasUncaughtExceptionCaptureCallback
+    } = NativeModule.require('internal/process/execution');
+
+    process._fatalException = fatalException;
+    process.setUncaughtExceptionCaptureCallback =
+      setUncaughtExceptionCaptureCallback;
+    process.hasUncaughtExceptionCaptureCallback =
+      hasUncaughtExceptionCaptureCallback;
+  }
 
   setupGlobalVariables();
 
@@ -83,9 +97,7 @@ function startup() {
     process.reallyExit = rawMethods.reallyExit;
     process._kill = rawMethods._kill;
 
-    const wrapped = perThreadSetup.wrapProcessMethods(
-      rawMethods, exceptionHandlerState
-    );
+    const wrapped = perThreadSetup.wrapProcessMethods(rawMethods);
     process._rawDebug = wrapped._rawDebug;
     process.hrtime = wrapped.hrtime;
     process.hrtime.bigint = wrapped.hrtimeBigInt;
@@ -93,10 +105,6 @@ function startup() {
     process.memoryUsage = wrapped.memoryUsage;
     process.kill = wrapped.kill;
     process.exit = wrapped.exit;
-    process.setUncaughtExceptionCaptureCallback =
-      wrapped.setUncaughtExceptionCaptureCallback;
-    process.hasUncaughtExceptionCaptureCallback =
-      wrapped.hasUncaughtExceptionCaptureCallback;
   }
 
   NativeModule.require('internal/process/warning').setup();
@@ -305,7 +313,7 @@ function startExecution() {
   // This means we are in a Worker context, and any script execution
   // will be directed by the worker module.
   if (internalBinding('worker').getEnvMessagePort() !== undefined) {
-    NativeModule.require('internal/worker').setupChild(evalScript);
+    NativeModule.require('internal/worker').setupChild();
     return;
   }
 
@@ -376,7 +384,9 @@ function executeUserCode() {
       addBuiltinLibsToObject
     } = NativeModule.require('internal/modules/cjs/helpers');
     addBuiltinLibsToObject(global);
-    evalScript('[eval]', wrapForBreakOnFirstLine(getOptionValue('--eval')));
+    const source = getOptionValue('--eval');
+    const { evalScript } = NativeModule.require('internal/process/execution');
+    evalScript('[eval]', source, process._breakFirstLine);
     return;
   }
 
@@ -430,7 +440,8 @@ function executeUserCode() {
 
     // User passed '-e' or '--eval' along with `-i` or `--interactive`
     if (process._eval != null) {
-      evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
+      const { evalScript } = NativeModule.require('internal/process/execution');
+      evalScript('[eval]', process._eval, process._breakFirstLine);
     }
     return;
   }
@@ -452,7 +463,8 @@ function readAndExecuteStdin() {
       checkScriptSyntax(code, '[stdin]');
     } else {
       process._eval = code;
-      evalScript('[stdin]', wrapForBreakOnFirstLine(process._eval));
+      const { evalScript } = NativeModule.require('internal/process/execution');
+      evalScript('[stdin]', process._eval, process._breakFirstLine);
     }
   });
 }
@@ -656,95 +668,6 @@ function setupDOMException() {
   registerDOMException(DOMException);
 }
 
-function noop() {}
-
-function setupProcessFatal() {
-  const {
-    executionAsyncId,
-    clearDefaultTriggerAsyncId,
-    clearAsyncIdStack,
-    hasAsyncIdStack,
-    afterHooksExist,
-    emitAfter
-  } = NativeModule.require('internal/async_hooks');
-
-  process._fatalException = (er) => {
-    // It's possible that defaultTriggerAsyncId was set for a constructor
-    // call that threw and was never cleared. So clear it now.
-    clearDefaultTriggerAsyncId();
-
-    if (exceptionHandlerState.captureFn !== null) {
-      exceptionHandlerState.captureFn(er);
-    } else if (!process.emit('uncaughtException', er)) {
-      // If someone handled it, then great.  otherwise, die in C++ land
-      // since that means that we'll exit the process, emit the 'exit' event.
-      try {
-        if (!process._exiting) {
-          process._exiting = true;
-          process.exitCode = 1;
-          process.emit('exit', 1);
-        }
-      } catch {
-        // Nothing to be done about it at this point.
-      }
-      try {
-        const { kExpandStackSymbol } = NativeModule.require('internal/util');
-        if (typeof er[kExpandStackSymbol] === 'function')
-          er[kExpandStackSymbol]();
-      } catch {
-        // Nothing to be done about it at this point.
-      }
-      return false;
-    }
-
-    // If we handled an error, then make sure any ticks get processed
-    // by ensuring that the next Immediate cycle isn't empty.
-    NativeModule.require('timers').setImmediate(noop);
-
-    // Emit the after() hooks now that the exception has been handled.
-    if (afterHooksExist()) {
-      do {
-        emitAfter(executionAsyncId());
-      } while (hasAsyncIdStack());
-    // Or completely empty the id stack.
-    } else {
-      clearAsyncIdStack();
-    }
-
-    return true;
-  };
-}
-
-function wrapForBreakOnFirstLine(source) {
-  if (!process._breakFirstLine)
-    return source;
-  const fn = `function() {\n\n${source};\n\n}`;
-  return `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
-}
-
-function evalScript(name, body) {
-  const CJSModule = NativeModule.require('internal/modules/cjs/loader');
-  const path = NativeModule.require('path');
-  const { tryGetCwd } = NativeModule.require('internal/util');
-  const cwd = tryGetCwd(path);
-
-  const module = new CJSModule(name);
-  module.filename = path.join(cwd, name);
-  module.paths = CJSModule._nodeModulePaths(cwd);
-  const script = `global.__filename = ${JSON.stringify(name)};\n` +
-                  'global.exports = exports;\n' +
-                  'global.module = module;\n' +
-                  'global.__dirname = __dirname;\n' +
-                  'global.require = require;\n' +
-                  'return require("vm").runInThisContext(' +
-                  `${JSON.stringify(body)}, { filename: ` +
-                  `${JSON.stringify(name)}, displayErrors: true });\n`;
-  const result = module._compile(script, `${name}-wrapper`);
-  if (getOptionValue('--print')) console.log(result);
-  // Handle any nextTicks added in the first tick of the program.
-  process._tickCallback();
-}
-
 function checkScriptSyntax(source, filename) {
   const CJSModule = NativeModule.require('internal/modules/cjs/loader');
   const vm = NativeModule.require('vm');
diff --git a/lib/internal/console/inspector.js b/lib/internal/console/inspector.js
index d481896e8a..3dbc68a193 100644
--- a/lib/internal/console/inspector.js
+++ b/lib/internal/console/inspector.js
@@ -1,15 +1,14 @@
 'use strict';
 
-const path = require('path');
 const CJSModule = require('internal/modules/cjs/loader');
 const { makeRequireFunction } = require('internal/modules/cjs/helpers');
-const { tryGetCwd } = require('internal/util');
+const { tryGetCwd } = require('internal/process/execution');
 const { addCommandLineAPI, consoleCall } = internalBinding('inspector');
 
 // Wrap a console implemented by Node.js with features from the VM inspector
 function addInspectorApis(consoleFromNode, consoleFromVM) {
   // Setup inspector command line API.
-  const cwd = tryGetCwd(path);
+  const cwd = tryGetCwd();
   const consoleAPIModule = new CJSModule('<inspector console>');
   consoleAPIModule.paths =
       CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
new file mode 100644
index 0000000000..607829544a
--- /dev/null
+++ b/lib/internal/process/execution.js
@@ -0,0 +1,149 @@
+'use strict';
+
+const path = require('path');
+
+const {
+  codes: {
+    ERR_INVALID_ARG_TYPE,
+    ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET
+  }
+} = require('internal/errors');
+
+const {
+  executionAsyncId,
+  clearDefaultTriggerAsyncId,
+  clearAsyncIdStack,
+  hasAsyncIdStack,
+  afterHooksExist,
+  emitAfter
+} = require('internal/async_hooks');
+
+// shouldAbortOnUncaughtToggle is a typed array for faster
+// communication with JS.
+const { shouldAbortOnUncaughtToggle } = internalBinding('util');
+
+function tryGetCwd() {
+  try {
+    return process.cwd();
+  } catch {
+    // getcwd(3) can fail if the current working directory has been deleted.
+    // Fall back to the directory name of the (absolute) executable path.
+    // It's not really correct but what are the alternatives?
+    return path.dirname(process.execPath);
+  }
+}
+
+function evalScript(name, body, breakFristLine) {
+  const CJSModule = require('internal/modules/cjs/loader');
+  if (breakFristLine) {
+    const fn = `function() {\n\n${body};\n\n}`;
+    body = `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
+  }
+
+  const cwd = tryGetCwd();
+
+  const module = new CJSModule(name);
+  module.filename = path.join(cwd, name);
+  module.paths = CJSModule._nodeModulePaths(cwd);
+  const script = `global.__filename = ${JSON.stringify(name)};\n` +
+                  'global.exports = exports;\n' +
+                  'global.module = module;\n' +
+                  'global.__dirname = __dirname;\n' +
+                  'global.require = require;\n' +
+                  'return require("vm").runInThisContext(' +
+                  `${JSON.stringify(body)}, { filename: ` +
+                  `${JSON.stringify(name)}, displayErrors: true });\n`;
+  const result = module._compile(script, `${name}-wrapper`);
+  if (require('internal/options').getOptionValue('--print')) {
+    console.log(result);
+  }
+  // Handle any nextTicks added in the first tick of the program.
+  process._tickCallback();
+}
+
+const exceptionHandlerState = { captureFn: null };
+
+function setUncaughtExceptionCaptureCallback(fn) {
+  if (fn === null) {
+    exceptionHandlerState.captureFn = fn;
+    shouldAbortOnUncaughtToggle[0] = 1;
+    return;
+  }
+  if (typeof fn !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE('fn', ['Function', 'null'], fn);
+  }
+  if (exceptionHandlerState.captureFn !== null) {
+    throw new ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET();
+  }
+  exceptionHandlerState.captureFn = fn;
+  shouldAbortOnUncaughtToggle[0] = 0;
+}
+
+function hasUncaughtExceptionCaptureCallback() {
+  return exceptionHandlerState.captureFn !== null;
+}
+
+function noop() {}
+
+// XXX(joyeecheung): for some reason this cannot be defined at the top-level
+// and exported to be written to process._fatalException, it has to be
+// returned as an *anonymous function* wrapped inside a factory function,
+// otherwise it breaks the test-timers.setInterval async hooks test -
+// this may indicate that node::FatalException should fix up the callback scope
+// before calling into process._fatalException, or this function should
+// take extra care of the async hooks before it schedules a setImmediate.
+function createFatalException() {
+  return (er) => {
+    // It's possible that defaultTriggerAsyncId was set for a constructor
+    // call that threw and was never cleared. So clear it now.
+    clearDefaultTriggerAsyncId();
+
+    if (exceptionHandlerState.captureFn !== null) {
+      exceptionHandlerState.captureFn(er);
+    } else if (!process.emit('uncaughtException', er)) {
+      // If someone handled it, then great.  otherwise, die in C++ land
+      // since that means that we'll exit the process, emit the 'exit' event.
+      try {
+        if (!process._exiting) {
+          process._exiting = true;
+          process.exitCode = 1;
+          process.emit('exit', 1);
+        }
+      } catch {
+        // Nothing to be done about it at this point.
+      }
+      try {
+        const { kExpandStackSymbol } = require('internal/util');
+        if (typeof er[kExpandStackSymbol] === 'function')
+          er[kExpandStackSymbol]();
+      } catch {
+        // Nothing to be done about it at this point.
+      }
+      return false;
+    }
+
+    // If we handled an error, then make sure any ticks get processed
+    // by ensuring that the next Immediate cycle isn't empty.
+    require('timers').setImmediate(noop);
+
+    // Emit the after() hooks now that the exception has been handled.
+    if (afterHooksExist()) {
+      do {
+        emitAfter(executionAsyncId());
+      } while (hasAsyncIdStack());
+    // Or completely empty the id stack.
+    } else {
+      clearAsyncIdStack();
+    }
+
+    return true;
+  };
+}
+
+module.exports = {
+  tryGetCwd,
+  evalScript,
+  fatalException: createFatalException(),
+  setUncaughtExceptionCaptureCallback,
+  hasUncaughtExceptionCaptureCallback
+};
diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js
index f1629e3a97..efa736fa61 100644
--- a/lib/internal/process/per_thread.js
+++ b/lib/internal/process/per_thread.js
@@ -12,7 +12,6 @@ const {
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_OPT_VALUE,
     ERR_OUT_OF_RANGE,
-    ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET,
     ERR_UNKNOWN_SIGNAL
   }
 } = require('internal/errors');
@@ -24,7 +23,7 @@ function assert(x, msg) {
 }
 
 // The execution of this function itself should not cause any side effects.
-function wrapProcessMethods(binding, exceptionHandlerState) {
+function wrapProcessMethods(binding) {
   const {
     hrtime: _hrtime,
     hrtimeBigInt: _hrtimeBigInt,
@@ -185,29 +184,6 @@ function wrapProcessMethods(binding, exceptionHandlerState) {
     return true;
   }
 
-  // shouldAbortOnUncaughtToggle is a typed array for faster
-  // communication with JS.
-  const { shouldAbortOnUncaughtToggle } = binding;
-
-  function setUncaughtExceptionCaptureCallback(fn) {
-    if (fn === null) {
-      exceptionHandlerState.captureFn = fn;
-      shouldAbortOnUncaughtToggle[0] = 1;
-      return;
-    }
-    if (typeof fn !== 'function') {
-      throw new ERR_INVALID_ARG_TYPE('fn', ['Function', 'null'], fn);
-    }
-    if (exceptionHandlerState.captureFn !== null) {
-      throw new ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET();
-    }
-    exceptionHandlerState.captureFn = fn;
-    shouldAbortOnUncaughtToggle[0] = 0;
-  }
-
-  function hasUncaughtExceptionCaptureCallback() {
-    return exceptionHandlerState.captureFn !== null;
-  }
 
   return {
     _rawDebug,
@@ -216,9 +192,7 @@ function wrapProcessMethods(binding, exceptionHandlerState) {
     cpuUsage,
     memoryUsage,
     kill,
-    exit,
-    setUncaughtExceptionCaptureCallback,
-    hasUncaughtExceptionCaptureCallback
+    exit
   };
 }
 
diff --git a/lib/internal/util.js b/lib/internal/util.js
index 902375afdb..81bc78bb94 100644
--- a/lib/internal/util.js
+++ b/lib/internal/util.js
@@ -387,17 +387,6 @@ function once(callback) {
   };
 }
 
-function tryGetCwd(path) {
-  try {
-    return process.cwd();
-  } catch {
-    // getcwd(3) can fail if the current working directory has been deleted.
-    // Fall back to the directory name of the (absolute) executable path.
-    // It's not really correct but what are the alternatives?
-    return path.dirname(process.execPath);
-  }
-}
-
 module.exports = {
   assertCrypto,
   cachedResult,
@@ -417,7 +406,6 @@ module.exports = {
   once,
   promisify,
   spliceOne,
-  tryGetCwd,
   removeColors,
 
   // Symbol used to customize promisify conversion
diff --git a/lib/internal/worker.js b/lib/internal/worker.js
index 9e41dba716..c079afbeb1 100644
--- a/lib/internal/worker.js
+++ b/lib/internal/worker.js
@@ -432,7 +432,7 @@ if (!isMainThread) {
 
 let originalFatalException;
 
-function setupChild(evalScript) {
+function setupChild() {
   // Called during bootstrap to set up worker script execution.
   debug(`[${threadId}] is setting up worker child environment`);
   const port = getEnvMessagePort();
@@ -453,6 +453,7 @@ function setupChild(evalScript) {
       port.unref();
       port.postMessage({ type: messageTypes.UP_AND_RUNNING });
       if (doEval) {
+        const { evalScript } = require('internal/process/execution');
         evalScript('[worker eval]', filename);
       } else {
         process.argv[1] = filename; // script filename