From 9a7e883b83687c31e340f105b590a72a7a02f3a5 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 1 Feb 2019 04:03:25 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/26000 Reviewed-By: Anna Henningsen Reviewed-By: Richard Lau Reviewed-By: Minwoo Jung --- lib/internal/bootstrap/node.js | 25 -------------- lib/internal/bootstrap/pre_execution.js | 42 +++++++++++++++++++++--- lib/internal/main/check_syntax.js | 12 ++----- lib/internal/main/eval_stdin.js | 12 ++----- lib/internal/main/eval_string.js | 12 ++----- lib/internal/main/repl.js | 12 ++----- lib/internal/main/run_main_module.js | 12 ++----- lib/internal/main/worker_thread.js | 22 +++++++++++-- lib/internal/process/main_thread_only.js | 14 -------- 9 files changed, 67 insertions(+), 96 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 338abcf55bb03e..78d04fe4446d5b 100644 --- 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 index d09fdb131a2ca9..ce4af115badaab 100644 --- 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 index 97584841b3a0c2..5d98701132f0e8 100644 --- 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 index f02d9ffa0cea03..2a2ef6d38a1fe8 100644 --- a/lib/internal/main/eval_stdin.js +++ b/lib/internal/main/eval_stdin.js @@ -3,11 +3,7 @@ // 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 { @@ -15,11 +11,7 @@ const { 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 index 7f746a6b11a951..953fab386d0671 100644 --- 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 index e931444ef32502..e6b98853511d11 100644 --- 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 index abc41fc20220f0..97a4e33be2e629 100644 --- 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 index 7e4466c24d0c31..ac161b7588f04c 100644 --- 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 index 0ce063f928974a..96c57fda35c755 100644 --- 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 };