From ca5b0c4f11c4f7ef7791153af6f1a968be17d079 Mon Sep 17 00:00:00 2001 From: cornholio <0@mcornholio.ru> Date: Mon, 10 Jul 2017 15:19:57 -0400 Subject: [PATCH 1/3] doc: fixes in cluster.md * Capitalization and punctuation. * `setupMaster` contained info about `settings` which where incomplete. PR-URL: https://github.com/nodejs/node/pull/14140 Fixes: https://github.com/nodejs/node/issues/8495 Fixes: https://github.com/nodejs/node/issues/12941 Refs: https://github.com/nodejs/node/pull/9659 Refs: https://github.com/nodejs/node/pull/13761 Reviewed-By: Refael Ackermann Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- doc/api/cluster.md | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/doc/api/cluster.md b/doc/api/cluster.md index e371583b3a7c28..85c3f2deb9fba1 100644 --- a/doc/api/cluster.md +++ b/doc/api/cluster.md @@ -700,12 +700,12 @@ changes: --> * {Object} - * `execArgv` {Array} list of string arguments passed to the Node.js + * `execArgv` {Array} List of string arguments passed to the Node.js executable. (Default=`process.execArgv`) - * `exec` {string} file path to worker file. (Default=`process.argv[1]`) - * `args` {Array} string arguments passed to worker. + * `exec` {string} File path to worker file. (Default=`process.argv[1]`) + * `args` {Array} String arguments passed to worker. (Default=`process.argv.slice(2)`) - * `silent` {boolean} whether or not to send output to parent's stdio. + * `silent` {boolean} Whether or not to send output to parent's stdio. (Default=`false`) * `stdio` {Array} Configures the stdio of forked processes. Because the cluster module relies on IPC to function, this configuration must contain an @@ -727,26 +727,19 @@ changes: description: The `stdio` option is supported now. --> -* `settings` {Object} - * `exec` {string} file path to worker file. (Default=`process.argv[1]`) - * `args` {Array} string arguments passed to worker. - (Default=`process.argv.slice(2)`) - * `silent` {boolean} whether or not to send output to parent's stdio. - (Default=`false`) - * `stdio` {Array} Configures the stdio of forked processes. When this option - is provided, it overrides `silent`. +* `settings` {Object} see [`cluster.settings`][] `setupMaster` is used to change the default 'fork' behavior. Once called, the settings will be present in `cluster.settings`. Note that: -* any settings changes only affect future calls to `.fork()` and have no - effect on workers that are already running +* Any settings changes only affect future calls to `.fork()` and have no + effect on workers that are already running. * The *only* attribute of a worker that cannot be set via `.setupMaster()` is - the `env` passed to `.fork()` -* the defaults above apply to the first call only, the defaults for later - calls is the current value at the time of `cluster.setupMaster()` is called + the `env` passed to `.fork()`. +* The defaults above apply to the first call only, the defaults for later + calls is the current value at the time of `cluster.setupMaster()` is called. Example: @@ -834,3 +827,4 @@ socket.on('data', (id) => { [Child Process module]: child_process.html#child_process_child_process_fork_modulepath_args_options [child_process event: 'exit']: child_process.html#child_process_event_exit [child_process event: 'message']: child_process.html#child_process_event_message +[`cluster.settings`]: #clustersettings From 592b0ed431cb4c4a1d6902bb8c68c112e0e61309 Mon Sep 17 00:00:00 2001 From: cornholio <0@mcornholio.ru> Date: Mon, 10 Jul 2017 22:23:22 +0300 Subject: [PATCH 2/3] test: reduce offset in test-inspector-port-cluster 10 ports for each test-case is too much. Not enough ports for new test cases, considering ~100 ports per file. PR-URL: https://github.com/nodejs/node/pull/14140 Fixes: https://github.com/nodejs/node/issues/8495 Fixes: https://github.com/nodejs/node/issues/12941 Refs: https://github.com/nodejs/node/pull/9659 Refs: https://github.com/nodejs/node/pull/13761 Reviewed-By: Refael Ackermann Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- test/inspector/test-inspector-port-cluster.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/inspector/test-inspector-port-cluster.js b/test/inspector/test-inspector-port-cluster.js index b2a53f87ea172e..f73c0fd3673969 100644 --- a/test/inspector/test-inspector-port-cluster.js +++ b/test/inspector/test-inspector-port-cluster.js @@ -23,7 +23,7 @@ function testRunnerMain() { workers: [{expectedPort: 9230}] }); - let port = debuggerPort + offset++ * 10; + let port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: [`--inspect=${port}`], @@ -34,28 +34,28 @@ function testRunnerMain() { ] }); - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: ['--inspect', `--inspect-port=${port}`], workers: [{expectedPort: port + 1}] }); - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: ['--inspect', `--debug-port=${port}`], workers: [{expectedPort: port + 1}] }); - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: [`--inspect=0.0.0.0:${port}`], workers: [{expectedPort: port + 1, expectedHost: '0.0.0.0'}] }); - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: [`--inspect=127.0.0.1:${port}`], @@ -63,14 +63,14 @@ function testRunnerMain() { }); if (common.hasIPv6) { - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: [`--inspect=[::]:${port}`], workers: [{expectedPort: port + 1, expectedHost: '::'}] }); - port = debuggerPort + offset++ * 10; + port = debuggerPort + offset++ * 5; spawnMaster({ execArgv: [`--inspect=[::1]:${port}`], From b4300536f5e77561137259105814e3a6c5d5121b Mon Sep 17 00:00:00 2001 From: cornholio <0@mcornholio.ru> Date: Mon, 10 Jul 2017 22:24:49 +0300 Subject: [PATCH 3/3] cluster: overriding inspector port Added an option to override inspector port for workers using `settings.inspectPort` will override default port incrementing behavior. Also, using this option allows to set 0 port for the whole cluster. PR-URL: https://github.com/nodejs/node/pull/14140 Fixes: https://github.com/nodejs/node/issues/8495 Fixes: https://github.com/nodejs/node/issues/12941 Refs: https://github.com/nodejs/node/pull/9659 Refs: https://github.com/nodejs/node/pull/13761 Reviewed-By: Refael Ackermann Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- doc/api/cluster.md | 3 + lib/internal/cluster/master.js | 20 +- test/inspector/test-inspector-port-cluster.js | 251 ++++++++++++++++-- 3 files changed, 251 insertions(+), 23 deletions(-) diff --git a/doc/api/cluster.md b/doc/api/cluster.md index 85c3f2deb9fba1..4f94bc87e50732 100644 --- a/doc/api/cluster.md +++ b/doc/api/cluster.md @@ -712,6 +712,9 @@ changes: `'ipc'` entry. When this option is provided, it overrides `silent`. * `uid` {number} Sets the user identity of the process. (See setuid(2).) * `gid` {number} Sets the group identity of the process. (See setgid(2).) + * `inspectPort` {number|function} Sets inspector port of worker. + Accepts number, or function that evaluates to number. By default + each worker gets port, incremented from master's `process.debugPort`. After calling `.setupMaster()` (or `.fork()`) this settings object will contain the settings, including the default values. diff --git a/lib/internal/cluster/master.js b/lib/internal/cluster/master.js index 2d1e2d3097f75b..05663e792b3ba6 100644 --- a/lib/internal/cluster/master.js +++ b/lib/internal/cluster/master.js @@ -12,6 +12,7 @@ const cluster = new EventEmitter(); const intercom = new EventEmitter(); const SCHED_NONE = 1; const SCHED_RR = 2; +const {isLegalPort} = require('internal/net'); module.exports = cluster; @@ -104,8 +105,23 @@ function createWorkerProcess(id, env) { workerEnv.NODE_UNIQUE_ID = '' + id; if (execArgv.some((arg) => arg.match(debugArgRegex))) { - execArgv.push(`--inspect-port=${process.debugPort + debugPortOffset}`); - debugPortOffset++; + let inspectPort; + if ('inspectPort' in cluster.settings) { + if (typeof cluster.settings.inspectPort === 'function') + inspectPort = cluster.settings.inspectPort(); + else + inspectPort = cluster.settings.inspectPort; + + if (!isLegalPort(inspectPort)) { + throw new TypeError('cluster.settings.inspectPort' + + ' is invalid'); + } + } else { + inspectPort = process.debugPort + debugPortOffset; + debugPortOffset++; + } + + execArgv.push(`--inspect-port=${inspectPort}`); } return fork(cluster.settings.exec, cluster.settings.args, { diff --git a/test/inspector/test-inspector-port-cluster.js b/test/inspector/test-inspector-port-cluster.js index f73c0fd3673969..166f5a8d108664 100644 --- a/test/inspector/test-inspector-port-cluster.js +++ b/test/inspector/test-inspector-port-cluster.js @@ -18,7 +18,7 @@ let offset = 0; */ function testRunnerMain() { - spawnMaster({ + let defaultPortCase = spawnMaster({ execArgv: ['--inspect'], workers: [{expectedPort: 9230}] }); @@ -77,42 +77,251 @@ function testRunnerMain() { workers: [{expectedPort: port + 1, expectedHost: '::1'}] }); } -} + // These tests check that setting inspectPort in cluster.settings + // would take effect and override port incrementing behavior + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: port + 2}, + workers: [{expectedPort: port + 2}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'addTwo'}, + workers: [ + {expectedPort: port + 2}, + {expectedPort: port + 4} + ] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'string'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'null'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'bignumber'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'negativenumber'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'bignumberfunc'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 'strfunc'}, + workers: [{}] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [], + clusterSettings: {inspectPort: port, execArgv: ['--inspect']}, + workers: [ + {expectedPort: port} + ] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [`--inspect=${port}`], + clusterSettings: {inspectPort: 0}, + workers: [ + {expectedInitialPort: 0}, + {expectedInitialPort: 0}, + {expectedInitialPort: 0} + ] + }); + + port = debuggerPort + offset++ * 5; + + spawnMaster({ + execArgv: [], + clusterSettings: {inspectPort: 0}, + workers: [ + {expectedInitialPort: 0}, + {expectedInitialPort: 0}, + {expectedInitialPort: 0} + ] + }); + + defaultPortCase.then(() => { + port = debuggerPort + offset++ * 5; + defaultPortCase = spawnMaster({ + execArgv: ['--inspect'], + clusterSettings: {inspectPort: port + 2}, + workers: [ + {expectedInitialPort: port + 2} + ] + }); + }); +} function masterProcessMain() { const workers = JSON.parse(process.env.workers); + const clusterSettings = JSON.parse(process.env.clusterSettings); + let debugPort = process.debugPort; for (const worker of workers) { - cluster.fork({ - expectedPort: worker.expectedPort, - expectedHost: worker.expectedHost - }).on('exit', common.mustCall(checkExitCode)); + const params = {}; + + if (worker.expectedPort) { + params.expectedPort = worker.expectedPort; + } + + if (worker.expectedInitialPort) { + params.expectedInitialPort = worker.expectedInitialPort; + } + + if (worker.expectedHost) { + params.expectedHost = worker.expectedHost; + } + + if (clusterSettings) { + if (clusterSettings.inspectPort === 'addTwo') { + clusterSettings.inspectPort = common.mustCall( + () => { return debugPort += 2; }, + workers.length + ); + } else if (clusterSettings.inspectPort === 'string') { + clusterSettings.inspectPort = 'string'; + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } else if (clusterSettings.inspectPort === 'null') { + clusterSettings.inspectPort = null; + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } else if (clusterSettings.inspectPort === 'bignumber') { + clusterSettings.inspectPort = 1293812; + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } else if (clusterSettings.inspectPort === 'negativenumber') { + clusterSettings.inspectPort = -9776; + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } else if (clusterSettings.inspectPort === 'bignumberfunc') { + clusterSettings.inspectPort = common.mustCall( + () => 123121, + workers.length + ); + + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } else if (clusterSettings.inspectPort === 'strfunc') { + clusterSettings.inspectPort = common.mustCall( + () => 'invalidPort', + workers.length + ); + + cluster.setupMaster(clusterSettings); + + assert.throws(() => { + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); + }, TypeError); + + return; + } + cluster.setupMaster(clusterSettings); + } + + cluster.fork(params).on('exit', common.mustCall(checkExitCode)); } } function workerProcessMain() { - const {expectedPort, expectedHost} = process.env; + const {expectedPort, expectedInitialPort, expectedHost} = process.env; + const debugOptions = process.binding('config').debugOptions; + + if ('expectedPort' in process.env) { + assert.strictEqual(process.debugPort, +expectedPort); + } - assert.strictEqual(process.debugPort, +expectedPort); + if ('expectedInitialPort' in process.env) { + assert.strictEqual(debugOptions.port, +expectedInitialPort); + } - if (expectedHost !== 'undefined') { - assert.strictEqual( - process.binding('config').debugOptions.host, - expectedHost - ); + if ('expectedHost' in process.env) { + assert.strictEqual(debugOptions.host, expectedHost); } process.exit(); } -function spawnMaster({execArgv, workers}) { - childProcess.fork(__filename, { - env: { - workers: JSON.stringify(workers), - testProcess: true - }, - execArgv - }).on('exit', common.mustCall(checkExitCode)); +function spawnMaster({execArgv, workers, clusterSettings = {}}) { + return new Promise((resolve) => { + childProcess.fork(__filename, { + env: { + workers: JSON.stringify(workers), + clusterSettings: JSON.stringify(clusterSettings), + testProcess: true + }, + execArgv + }).on('exit', common.mustCall((code, signal) => { + checkExitCode(code, signal); + resolve(); + })); + }); } function checkExitCode(code, signal) {