src: move more process methods initialization in bootstrap/node.js

Instead of:

- Writing methods onto the process directly in C++ during
  `SetupProcessObject()` and overwrite with argument checks later
- Or, wrapping and writing them in `internal/process/*.js`

Do:

- Move the C++ implementations in node_process.cc and mark them static
  wherever possible
- Expose the C++ methods through a new
  `internalBinding('process_methods')`
- Wrap the methods in `internal/process/*.js` in a
  side-effect-free manner and return them back to
  `internal/bootstrap/node.js`
- Centralize the write to the process object based on conditions
  in `bootstrap/node.js`

So it's easier to see what methods are attached to the process object
during bootstrap under what condition and in what order.

The eventual goal is to figure out the dependency of process methods
and the write/read access to the process object during bootstrap, group
these access properly and remove the process properties that should not
be exposed to users this way.

Also correct the NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE milestone
which should be marked before code execution.

Refs: https://github.com/nodejs/node/issues/24961

PR-URL: https://github.com/nodejs/node/pull/25127
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

3 files changed, 147 insertions, 118 deletions

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index c51961e9ce..c375374164 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -20,11 +20,7 @@
 const {
   _setupTraceCategoryState,
   _setupNextTick,
-  _setupPromises, _chdir, _cpuUsage,
-  _hrtime, _hrtimeBigInt,
-  _memoryUsage, _rawDebug,
-  _umask,
-  _shouldAbortOnUncaughtToggle
+  _setupPromises
 } = bootstrappers;
 const { internalBinding, NativeModule } = loaderExports;
 
@@ -57,15 +53,57 @@ function startup() {
     );
   }
 
-  perThreadSetup.setupAssert();
-  perThreadSetup.setupConfig();
+  // process.config is serialized config.gypi
+  process.config = JSON.parse(internalBinding('native_module').config);
 
+  const rawMethods = internalBinding('process_methods');
+  // Set up methods and events on the process object for the main thread
   if (isMainThread) {
+    // This depends on process being an event emitter
     mainThreadSetup.setupSignalHandlers(internalBinding);
+
+    process.abort = rawMethods.abort;
+    const wrapped = mainThreadSetup.wrapProcessMethods(rawMethods);
+    process.umask = wrapped.umask;
+    process.chdir = wrapped.chdir;
+
+    // TODO(joyeecheung): deprecate and remove these underscore methods
+    process._debugProcess = rawMethods._debugProcess;
+    process._debugEnd = rawMethods._debugEnd;
+    process._startProfilerIdleNotifier =
+      rawMethods._startProfilerIdleNotifier;
+    process._stopProfilerIdleNotifier = rawMethods._stopProfilerIdleNotifier;
   }
 
-  perThreadSetup.setupUncaughtExceptionCapture(exceptionHandlerState,
-                                               _shouldAbortOnUncaughtToggle);
+  // Set up methods on the process object for all threads
+  {
+    process.cwd = rawMethods.cwd;
+    process.dlopen = rawMethods.dlopen;
+    process.uptime = rawMethods.uptime;
+
+    // TODO(joyeecheung): either remove them or make them public
+    process._getActiveRequests = rawMethods._getActiveRequests;
+    process._getActiveHandles = rawMethods._getActiveHandles;
+
+    // TODO(joyeecheung): remove these
+    process.reallyExit = rawMethods.reallyExit;
+    process._kill = rawMethods._kill;
+
+    const wrapped = perThreadSetup.wrapProcessMethods(
+      rawMethods, exceptionHandlerState
+    );
+    process._rawDebug = wrapped._rawDebug;
+    process.hrtime = wrapped.hrtime;
+    process.hrtime.bigint = wrapped.hrtimeBigInt;
+    process.cpuUsage = wrapped.cpuUsage;
+    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();
   NativeModule.require('internal/process/next_tick').setup(_setupNextTick,
@@ -91,22 +129,10 @@ function startup() {
 
   if (isMainThread) {
     mainThreadSetup.setupStdio();
-    mainThreadSetup.setupProcessMethods(_chdir, _umask);
   } else {
     workerThreadSetup.setupStdio();
   }
 
-  const perf = internalBinding('performance');
-  const {
-    NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,
-  } = perf.constants;
-
-  perThreadSetup.setupRawDebug(_rawDebug);
-  perThreadSetup.setupHrtime(_hrtime, _hrtimeBigInt);
-  perThreadSetup.setupCpuUsage(_cpuUsage);
-  perThreadSetup.setupMemoryUsage(_memoryUsage);
-  perThreadSetup.setupKillAndExit();
-
   if (global.__coverage__)
     NativeModule.require('internal/process/write-coverage').setup();
 
@@ -209,9 +235,37 @@ function startup() {
     }
   }
 
-  perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
+  // process.allowedNodeEnvironmentFlags
+  Object.defineProperty(process, 'allowedNodeEnvironmentFlags', {
+    get() {
+      const flags = perThreadSetup.buildAllowedFlags();
+      process.allowedNodeEnvironmentFlags = flags;
+      return process.allowedNodeEnvironmentFlags;
+    },
+    // If the user tries to set this to another value, override
+    // this completely to that value.
+    set(value) {
+      Object.defineProperty(this, 'allowedNodeEnvironmentFlags', {
+        value,
+        configurable: true,
+        enumerable: true,
+        writable: true
+      });
+    },
+    enumerable: true,
+    configurable: true
+  });
+  // process.assert
+  process.assert = deprecate(
+    perThreadSetup.assert,
+    'process.assert() is deprecated. Please use the `assert` module instead.',
+    'DEP0100');
 
-  perThreadSetup.setupAllowedFlags();
+  const perf = internalBinding('performance');
+  const {
+    NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,
+  } = perf.constants;
+  perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
 
   startExecution();
 }
diff --git a/lib/internal/process/main_thread_only.js b/lib/internal/process/main_thread_only.js
index 5f53f34084..1db693f1ec 100644
--- a/lib/internal/process/main_thread_only.js
+++ b/lib/internal/process/main_thread_only.js
@@ -27,21 +27,25 @@ function setupStdio() {
   setupProcessStdio(getMainThreadStdio());
 }
 
-// Non-POSIX platforms like Windows don't have certain methods.
-// Workers also lack these methods since they change process-global state.
-function setupProcessMethods(_chdir, _umask) {
-  process.chdir = function chdir(directory) {
+// The execution of this function itself should not cause any side effects.
+function wrapProcessMethods(binding) {
+  function chdir(directory) {
     validateString(directory, 'directory');
-    return _chdir(directory);
-  };
+    return binding.chdir(directory);
+  }
 
-  process.umask = function umask(mask) {
+  function umask(mask) {
     if (mask === undefined) {
       // Get the mask
-      return _umask(mask);
+      return binding.umask(mask);
     }
     mask = validateMode(mask, 'mask');
-    return _umask(mask);
+    return binding.umask(mask);
+  }
+
+  return {
+    chdir,
+    umask
   };
 }
 
@@ -175,7 +179,7 @@ function setupChildProcessIpcChannel() {
 
 module.exports = {
   setupStdio,
-  setupProcessMethods,
+  wrapProcessMethods,
   setupSignalHandlers,
   setupChildProcessIpcChannel,
   wrapPosixCredentialSetters
diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js
index 57cc9c3814..f1629e3a97 100644
--- a/lib/internal/process/per_thread.js
+++ b/lib/internal/process/per_thread.js
@@ -18,25 +18,30 @@ const {
 } = require('internal/errors');
 const util = require('util');
 const constants = internalBinding('constants').os.signals;
-const { deprecate } = require('internal/util');
-
-function setupAssert() {
-  process.assert = deprecate(
-    function(x, msg) {
-      if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
-    },
-    'process.assert() is deprecated. Please use the `assert` module instead.',
-    'DEP0100');
+
+function assert(x, msg) {
+  if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
 }
 
-// Set up the process.cpuUsage() function.
-function setupCpuUsage(_cpuUsage) {
+// The execution of this function itself should not cause any side effects.
+function wrapProcessMethods(binding, exceptionHandlerState) {
+  const {
+    hrtime: _hrtime,
+    hrtimeBigInt: _hrtimeBigInt,
+    cpuUsage: _cpuUsage,
+    memoryUsage: _memoryUsage
+  } = binding;
+
+  function _rawDebug(...args) {
+    binding._rawDebug(util.format.apply(null, args));
+  }
+
   // Create the argument array that will be passed to the native function.
   const cpuValues = new Float64Array(2);
 
   // Replace the native function with the JS version that calls the native
   // function.
-  process.cpuUsage = function cpuUsage(prevValue) {
+  function cpuUsage(prevValue) {
     // If a previous value was passed in, ensure it has the correct shape.
     if (prevValue) {
       if (!previousValueIsValid(prevValue.user)) {
@@ -80,7 +85,7 @@ function setupCpuUsage(_cpuUsage) {
       user: cpuValues[0],
       system: cpuValues[1]
     };
-  };
+  }
 
   // Ensure that a previously passed in value is valid. Currently, the native
   // implementation always returns numbers <= Number.MAX_SAFE_INTEGER.
@@ -89,15 +94,13 @@ function setupCpuUsage(_cpuUsage) {
         num <= Number.MAX_SAFE_INTEGER &&
         num >= 0;
   }
-}
 
-// The 3 entries filled in by the original process.hrtime contains
-// the upper/lower 32 bits of the second part of the value,
-// and the remaining nanoseconds of the value.
-function setupHrtime(_hrtime, _hrtimeBigInt) {
+  // The 3 entries filled in by the original process.hrtime contains
+  // the upper/lower 32 bits of the second part of the value,
+  // and the remaining nanoseconds of the value.
   const hrValues = new Uint32Array(3);
 
-  process.hrtime = function hrtime(time) {
+  function hrtime(time) {
     _hrtime(hrValues);
 
     if (time !== undefined) {
@@ -118,21 +121,18 @@ function setupHrtime(_hrtime, _hrtimeBigInt) {
       hrValues[0] * 0x100000000 + hrValues[1],
       hrValues[2]
     ];
-  };
+  }
 
   // Use a BigUint64Array in the closure because V8 does not have an API for
   // creating a BigInt out of a uint64_t yet.
   const hrBigintValues = new BigUint64Array(1);
-  process.hrtime.bigint = function() {
+  function hrtimeBigInt() {
     _hrtimeBigInt(hrBigintValues);
     return hrBigintValues[0];
-  };
-}
+  }
 
-function setupMemoryUsage(_memoryUsage) {
   const memValues = new Float64Array(4);
-
-  process.memoryUsage = function memoryUsage() {
+  function memoryUsage() {
     _memoryUsage(memValues);
     return {
       rss: memValues[0],
@@ -140,18 +140,9 @@ function setupMemoryUsage(_memoryUsage) {
       heapUsed: memValues[2],
       external: memValues[3]
     };
-  };
-}
-
-function setupConfig() {
-  // Serialized config.gypi
-  process.config = JSON.parse(internalBinding('native_module').config);
-}
-
-
-function setupKillAndExit() {
+  }
 
-  process.exit = function(code) {
+  function exit(code) {
     if (code || code === 0)
       process.exitCode = code;
 
@@ -159,10 +150,10 @@ function setupKillAndExit() {
       process._exiting = true;
       process.emit('exit', process.exitCode || 0);
     }
-    process.reallyExit(process.exitCode || 0);
-  };
+    binding.reallyExit(process.exitCode || 0);
+  }
 
-  process.kill = function(pid, sig) {
+  function kill(pid, sig) {
     var err;
     if (process.env.NODE_V8_COVERAGE) {
       const { writeCoverage } = require('internal/process/coverage');
@@ -176,6 +167,8 @@ function setupKillAndExit() {
 
     // preserve null signal
     if (sig === (sig | 0)) {
+      // XXX(joyeecheung): we have to use process._kill here because
+      // it's monkey-patched by tests.
       err = process._kill(pid, sig);
     } else {
       sig = sig || 'SIGTERM';
@@ -190,22 +183,13 @@ function setupKillAndExit() {
       throw errnoException(err, 'kill');
 
     return true;
-  };
-}
-
-function setupRawDebug(_rawDebug) {
-  process._rawDebug = function() {
-    _rawDebug(util.format.apply(null, arguments));
-  };
-}
-
+  }
 
-function setupUncaughtExceptionCapture(exceptionHandlerState,
-                                       shouldAbortOnUncaughtToggle) {
   // shouldAbortOnUncaughtToggle is a typed array for faster
   // communication with JS.
+  const { shouldAbortOnUncaughtToggle } = binding;
 
-  process.setUncaughtExceptionCaptureCallback = function(fn) {
+  function setUncaughtExceptionCaptureCallback(fn) {
     if (fn === null) {
       exceptionHandlerState.captureFn = fn;
       shouldAbortOnUncaughtToggle[0] = 1;
@@ -219,10 +203,22 @@ function setupUncaughtExceptionCapture(exceptionHandlerState,
     }
     exceptionHandlerState.captureFn = fn;
     shouldAbortOnUncaughtToggle[0] = 0;
-  };
+  }
 
-  process.hasUncaughtExceptionCaptureCallback = function() {
+  function hasUncaughtExceptionCaptureCallback() {
     return exceptionHandlerState.captureFn !== null;
+  }
+
+  return {
+    _rawDebug,
+    hrtime,
+    hrtimeBigInt,
+    cpuUsage,
+    memoryUsage,
+    kill,
+    exit,
+    setUncaughtExceptionCaptureCallback,
+    hasUncaughtExceptionCaptureCallback
   };
 }
 
@@ -326,38 +322,13 @@ function buildAllowedFlags() {
   Object.freeze(NodeEnvironmentFlagsSet.prototype.constructor);
   Object.freeze(NodeEnvironmentFlagsSet.prototype);
 
-  return process.allowedNodeEnvironmentFlags = Object.freeze(
-    new NodeEnvironmentFlagsSet(
-      allowedNodeEnvironmentFlags
-    ));
-}
-
-function setupAllowedFlags() {
-  Object.defineProperty(process, 'allowedNodeEnvironmentFlags', {
-    get: buildAllowedFlags,
-    set(value) {
-      // If the user tries to set this to another value, override
-      // this completely to that value.
-      Object.defineProperty(this, 'allowedNodeEnvironmentFlags', {
-        value,
-        configurable: true,
-        enumerable: true,
-        writable: true
-      });
-    },
-    enumerable: true,
-    configurable: true
-  });
+  return Object.freeze(new NodeEnvironmentFlagsSet(
+    allowedNodeEnvironmentFlags
+  ));
 }
 
 module.exports = {
-  setupAllowedFlags,
-  setupAssert,
-  setupCpuUsage,
-  setupHrtime,
-  setupMemoryUsage,
-  setupConfig,
-  setupKillAndExit,
-  setupRawDebug,
-  setupUncaughtExceptionCapture
+  assert,
+  buildAllowedFlags,
+  wrapProcessMethods
 };