process: group main thread execution preparation code

This patch groups the preparation of main thread execution into `prepareMainThreadExecution()` and removes the cluster IPC setup in worker thread bootstrap since clusters do not use worker threads for its implementation and it's unlikely to change. PR-URL: #26000 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
nodejs · Feb 10, 2019 · 9a7e883 · 9a7e883
1 parent d7bf070
commit 9a7e883
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 96 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -176,31 +176,6 @@ if (config.hasInspector) {
   internalBinding('inspector').registerAsyncHook(enable, disable);
 }
 
-// If the process is spawned with env NODE_CHANNEL_FD, it's probably
-// spawned by our child_process module, then initialize IPC.
-// This attaches some internal event listeners and creates:
-// process.send(), process.channel, process.connected,
-// process.disconnect()
-if (process.env.NODE_CHANNEL_FD) {
-  if (ownsProcessState) {
-    mainThreadSetup.setupChildProcessIpcChannel();
-  } else {
-    Object.defineProperty(process, 'channel', {
-      enumerable: false,
-      get: workerThreadSetup.unavailable('process.channel')
-    });
-
-    Object.defineProperty(process, 'connected', {
-      enumerable: false,
-      get: workerThreadSetup.unavailable('process.connected')
-    });
-
-    process.send = workerThreadSetup.unavailable('process.send()');
-    process.disconnect =
-      workerThreadSetup.unavailable('process.disconnect()');
-  }
-}
-
 const browserGlobals = !process._noBrowserGlobals;
 if (browserGlobals) {
   setupGlobalTimeouts();

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -2,6 +2,27 @@
 
 const { getOptionValue } = require('internal/options');
 
+function prepareMainThreadExecution() {
+  // If the process is spawned with env NODE_CHANNEL_FD, it's probably
+  // spawned by our child_process module, then initialize IPC.
+  // This attaches some internal event listeners and creates:
+  // process.send(), process.channel, process.connected,
+  // process.disconnect().
+  setupChildProcessIpcChannel();
+
+  // Load policy from disk and parse it.
+  initializePolicy();
+
+  // If this is a worker in cluster mode, start up the communication
+  // channel. This needs to be done before any user code gets executed
+  // (including preload modules).
+  initializeClusterIPC();
+
+  initializeDeprecations();
+  initializeESMLoader();
+  loadPreloadModules();
+}
+
 // In general deprecations are intialized wherever the APIs are implemented,
 // this is used to deprecate APIs implemented in C++ where the deprecation
 // utitlities are not easily accessible.
@@ -41,10 +62,22 @@ function initializeDeprecations() {
   }
 }
 
+function setupChildProcessIpcChannel() {
+  if (process.env.NODE_CHANNEL_FD) {
+    const assert = require('internal/assert');
+
+    const fd = parseInt(process.env.NODE_CHANNEL_FD, 10);
+    assert(fd >= 0);
+
+    // Make sure it's not accidentally inherited by child processes.
+    delete process.env.NODE_CHANNEL_FD;
+
+    require('child_process')._forkChild(fd);
+    assert(process.send);
+  }
+}
+
 function initializeClusterIPC() {
-  // If this is a worker in cluster mode, start up the communication
-  // channel. This needs to be done before any user code gets executed
-  // (including preload modules).
   if (process.argv[1] && process.env.NODE_UNIQUE_ID) {
     const cluster = require('cluster');
     cluster._setupWorker();
@@ -114,9 +147,8 @@ function loadPreloadModules() {
 }
 
 module.exports = {
+  prepareMainThreadExecution,
   initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
   initializeESMLoader,
   loadPreloadModules
 };
diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js
@@ -4,11 +4,7 @@
 // instead of actually running the file.
 
 const {
-  initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
-  initializeESMLoader,
-  loadPreloadModules
+  prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 
 const {
@@ -22,11 +18,7 @@ const {
 } = require('internal/modules/cjs/helpers');
 
 // TODO(joyeecheung): not every one of these are necessary
-initializeDeprecations();
-initializeClusterIPC();
-initializePolicy();
-initializeESMLoader();
-loadPreloadModules();
+prepareMainThreadExecution();
 markBootstrapComplete();
 
 if (process.argv[1] && process.argv[1] !== '-') {

diff --git a/lib/internal/main/eval_stdin.js b/lib/internal/main/eval_stdin.js
@@ -3,23 +3,15 @@
 // Stdin is not a TTY, we will read it and execute it.
 
 const {
-  initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
-  initializeESMLoader,
-  loadPreloadModules
+  prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 
 const {
   evalScript,
   readStdin
 } = require('internal/process/execution');
 
-initializeDeprecations();
-initializeClusterIPC();
-initializePolicy();
-initializeESMLoader();
-loadPreloadModules();
+prepareMainThreadExecution();
 markBootstrapComplete();
 
 readStdin((code) => {

diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js
@@ -4,21 +4,13 @@
 // `--interactive`.
 
 const {
-  initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
-  initializeESMLoader,
-  loadPreloadModules
+  prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 const { evalScript } = require('internal/process/execution');
 const { addBuiltinLibsToObject } = require('internal/modules/cjs/helpers');
 
 const source = require('internal/options').getOptionValue('--eval');
-initializeDeprecations();
-initializeClusterIPC();
-initializePolicy();
-initializeESMLoader();
-loadPreloadModules();
+prepareMainThreadExecution();
 addBuiltinLibsToObject(global);
 markBootstrapComplete();
 evalScript('[eval]', source, process._breakFirstLine);
diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js
@@ -4,22 +4,14 @@
 // the main module is not specified and stdin is a TTY.
 
 const {
-  initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
-  initializeESMLoader,
-  loadPreloadModules
+  prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 
 const {
   evalScript
 } = require('internal/process/execution');
 
-initializeDeprecations();
-initializeClusterIPC();
-initializePolicy();
-initializeESMLoader();
-loadPreloadModules();
+prepareMainThreadExecution();
 
 const cliRepl = require('internal/repl');
 cliRepl.createInternalRepl(process.env, (err, repl) => {

diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js
@@ -1,18 +1,10 @@
 'use strict';
 
 const {
-  initializeDeprecations,
-  initializeClusterIPC,
-  initializePolicy,
-  initializeESMLoader,
-  loadPreloadModules
+  prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 
-initializeDeprecations();
-initializeClusterIPC();
-initializePolicy();
-initializeESMLoader();
-loadPreloadModules();
+prepareMainThreadExecution();
 
 // Expand process.argv[1] into a full path.
 const path = require('path');

diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -5,7 +5,6 @@
 
 const {
   initializeDeprecations,
-  initializeClusterIPC,
   initializeESMLoader,
   loadPreloadModules
 } = require('internal/bootstrap/pre_execution');
@@ -42,6 +41,26 @@ debug(`[${threadId}] is setting up worker child environment`);
 // Set up the message port and start listening
 const port = getEnvMessagePort();
 
+// If the main thread is spawned with env NODE_CHANNEL_FD, it's probably
+// spawned by our child_process module. In the work threads, mark the
+// related IPC properties as unavailable.
+if (process.env.NODE_CHANNEL_FD) {
+  const workerThreadSetup = require('internal/process/worker_thread_only');
+  Object.defineProperty(process, 'channel', {
+    enumerable: false,
+    get: workerThreadSetup.unavailable('process.channel')
+  });
+
+  Object.defineProperty(process, 'connected', {
+    enumerable: false,
+    get: workerThreadSetup.unavailable('process.connected')
+  });
+
+  process.send = workerThreadSetup.unavailable('process.send()');
+  process.disconnect =
+    workerThreadSetup.unavailable('process.disconnect()');
+}
+
 port.on('message', (message) => {
   if (message.type === LOAD_SCRIPT) {
     const {
@@ -57,7 +76,6 @@ port.on('message', (message) => {
       require('internal/process/policy').setup(manifestSrc, manifestURL);
     }
     initializeDeprecations();
-    initializeClusterIPC();
     initializeESMLoader();
     loadPreloadModules();
     publicWorker.parentPort = publicPort;

diff --git a/lib/internal/process/main_thread_only.js b/lib/internal/process/main_thread_only.js
@@ -152,22 +152,8 @@ function createSignalHandlers() {
   };
 }
 
-function setupChildProcessIpcChannel() {
-  const assert = require('internal/assert');
-
-  const fd = parseInt(process.env.NODE_CHANNEL_FD, 10);
-  assert(fd >= 0);
-
-  // Make sure it's not accidentally inherited by child processes.
-  delete process.env.NODE_CHANNEL_FD;
-
-  require('child_process')._forkChild(fd);
-  assert(process.send);
-}
-
 module.exports = {
   wrapProcessMethods,
   createSignalHandlers,
-  setupChildProcessIpcChannel,
   wrapPosixCredentialSetters
 };