From f00765bd47d75b6cee69dd8b2b4379abc775877c Mon Sep 17 00:00:00 2001 From: cornholio <0@mcornholio.ru> Date: Mon, 28 Nov 2016 15:30:05 +0300 Subject: [PATCH 1/2] cluster: fixing debug ports for forking workers Fixing setting debug port numbers for workers in cluster Cases when master started without debug should work properly now --- lib/internal/cluster/master.js | 22 ++++++-- .../test-cluster-inspect-port-manual.js | 56 +++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 test/sequential/test-cluster-inspect-port-manual.js diff --git a/lib/internal/cluster/master.js b/lib/internal/cluster/master.js index b20a27c5ee5129..148c53edef5cd5 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 debugArgRegex = /^(--inspect(?:-brk|-port)?|--debug-port)(?:=(.*))?$/; module.exports = cluster; @@ -24,7 +25,7 @@ cluster.SCHED_NONE = SCHED_NONE; // Leave it to the operating system. cluster.SCHED_RR = SCHED_RR; // Master distributes connections. var ids = 0; -var debugPortOffset = 1; +var debugPortOffset = 0; var initialized = false; // XXX(bnoordhuis) Fold cluster.schedulingPolicy into cluster.settings? @@ -70,6 +71,15 @@ cluster.setupMaster = function(options) { assert(schedulingPolicy === SCHED_NONE || schedulingPolicy === SCHED_RR, `Bad cluster.schedulingPolicy: ${schedulingPolicy}`); + const hasDebugArg = process.execArgv.some((argv) => { + return debugArgRegex.test(argv); + }); + + if (hasDebugArg) { + // Increase debugPortOffset only if master was started with debug + debugPortOffset = 1; + } + process.nextTick(setupSettingsNT, settings); process.on('internalMessage', (message) => { @@ -104,13 +114,15 @@ function createWorkerProcess(id, env) { workerEnv.NODE_UNIQUE_ID = '' + id; for (var i = 0; i < execArgv.length; i++) { - const match = execArgv[i].match( - /^(--inspect|--inspect-(brk|port)|--debug|--debug-(brk|port))(=\d+)?$/ - ); + const match = execArgv[i].match(debugArgRegex); if (match) { if (debugPort === 0) { - debugPort = process.debugPort + debugPortOffset; + if (match[2] && match[2] !== '0') + debugPort = +match[2] + debugPortOffset; + else { + debugPort = process.debugPort + debugPortOffset; + } ++debugPortOffset; } diff --git a/test/sequential/test-cluster-inspect-port-manual.js b/test/sequential/test-cluster-inspect-port-manual.js new file mode 100644 index 00000000000000..fbc9211047a774 --- /dev/null +++ b/test/sequential/test-cluster-inspect-port-manual.js @@ -0,0 +1,56 @@ +'use strict'; + +/** + * This test suite checks debug port numbers + * when node started without --inspect argument + * and debug initiated with cluster.settings.execArgv or similar + */ + +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); + +if (cluster.isMaster) { + let message = 'If master is not debugging, ' + + 'forked worker should not have --inspect argument'; + + cluster.fork({message}).on('exit', common.mustCall(checkExitCode)); + + message = 'When debugging port is set at cluster.settings, ' + + 'forked worker should have debugPort without offset'; + cluster.settings.execArgv = ['--inspect=' + common.PORT]; + cluster.fork({ + portSet: common.PORT, + message + }).on('exit', common.mustCall(checkExitCode)); + + message = 'When using custom port number, ' + + 'forked worker should have debugPort incremented from that port'; + cluster.fork({ + portSet: common.PORT + 1, + message + }).on('exit', common.mustCall(checkExitCode)); + + cluster.fork({ + portSet: common.PORT + 2, + message + }).on('exit', common.mustCall(checkExitCode)); +} else { + const hasDebugArg = process.execArgv.some(function(arg) { + return /--inspect/.test(arg); + }); + + assert.strictEqual(hasDebugArg, process.env.portSet !== undefined); + hasDebugArg && assert.strictEqual( + process.debugPort, + +process.env.portSet, + process.env.message + + `; expected port: ${+process.env.portSet}, actual: ${process.debugPort}` + ); + process.exit(); +} + +function checkExitCode(code, signal) { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} From 48e3720ea167ba9da46e5b6562bec08fbd34c619 Mon Sep 17 00:00:00 2001 From: cornholio <0@mcornholio.ru> Date: Mon, 28 Nov 2016 15:31:18 +0300 Subject: [PATCH 2/2] cluster: better debug argument parsing Node supports debug option with --debug=host:port format. This commit fixes debug detection in cluster for those cases. Also, in order to change debug port you need to change only last argument. Thus, I've reversed the cycle and added break. --- lib/internal/cluster/master.js | 82 ++++++++-- .../test-cluster-inspector-debug-port.js | 41 ----- .../test-cluster-inspector-debug-port.js | 141 ++++++++++++++++++ 3 files changed, 209 insertions(+), 55 deletions(-) delete mode 100644 test/parallel/test-cluster-inspector-debug-port.js create mode 100644 test/sequential/test-cluster-inspector-debug-port.js diff --git a/lib/internal/cluster/master.js b/lib/internal/cluster/master.js index 148c53edef5cd5..ef2ef8696f81aa 100644 --- a/lib/internal/cluster/master.js +++ b/lib/internal/cluster/master.js @@ -12,8 +12,17 @@ const cluster = new EventEmitter(); const intercom = new EventEmitter(); const SCHED_NONE = 1; const SCHED_RR = 2; +let debugPortOffset = 0; + const debugArgRegex = /^(--inspect(?:-brk|-port)?|--debug-port)(?:=(.*))?$/; +const hasDebugArg = process.execArgv.some((argv) => debugArgRegex.test(argv)); + +if (hasDebugArg) { + // Increase debugPortOffset only if master was started with debug. + debugPortOffset = 1; +} + module.exports = cluster; cluster.isWorker = false; @@ -25,7 +34,6 @@ cluster.SCHED_NONE = SCHED_NONE; // Leave it to the operating system. cluster.SCHED_RR = SCHED_RR; // Master distributes connections. var ids = 0; -var debugPortOffset = 0; var initialized = false; // XXX(bnoordhuis) Fold cluster.schedulingPolicy into cluster.settings? @@ -106,27 +114,73 @@ function setupSettingsNT(settings) { } function createWorkerProcess(id, env) { - var workerEnv = util._extend({}, process.env); - var execArgv = cluster.settings.execArgv.slice(); - var debugPort = 0; + // If master process started with debug, workers should use ports with offset + // This code parses execArgv from cluster.settings + // and augments values of ports in following flags: + // --inspect, --inspect-brk, --inspect-port, --debug-port. + + const workerEnv = util._extend({}, process.env); + const execArgv = cluster.settings.execArgv.slice(); + const ipv6Regex = /^(\[[\d:]+\])(?::(.+))?$/; util._extend(workerEnv, env); workerEnv.NODE_UNIQUE_ID = '' + id; - for (var i = 0; i < execArgv.length; i++) { - const match = execArgv[i].match(debugArgRegex); + // Cycle is reversed on purpose: + // node currently selects debug port from last argument + // so we're changing last argument and breaking the cycle. + let i = execArgv.length; + while (i--) { + if (debugArgRegex.test(execArgv[i])) { + let debugPort = 0; + let debugHost = ''; + + const splitDebugArg = execArgv[i].split('='); + + let resultArg = splitDebugArg[0] + '='; + const debugSocket = splitDebugArg[1]; + + if (debugSocket !== undefined) { + const ipv6Match = debugSocket.match(ipv6Regex); + + if (ipv6Match) { + debugHost = ipv6Match[1]; + + if (ipv6Match[2]) { + debugPort = +ipv6Match[2]; + } + } else { + const splitDebugSocket = debugSocket.split(':'); + + if (splitDebugSocket[1]) { + debugHost = splitDebugSocket[0]; + debugPort = +splitDebugSocket[1]; + } else { + if (/^\d+$/.test(debugSocket)) { + debugPort = +debugSocket; + } else { + debugHost = debugSocket; + } + } + } + } - if (match) { if (debugPort === 0) { - if (match[2] && match[2] !== '0') - debugPort = +match[2] + debugPortOffset; - else { - debugPort = process.debugPort + debugPortOffset; - } - ++debugPortOffset; + debugPort = process.debugPort; + } + + debugPort += debugPortOffset; + ++debugPortOffset; + + if (debugHost) { + resultArg += debugHost + ':'; } - execArgv[i] = match[1] + '=' + debugPort; + resultArg += debugPort; + + execArgv[i] = resultArg; + + break; } } diff --git a/test/parallel/test-cluster-inspector-debug-port.js b/test/parallel/test-cluster-inspector-debug-port.js deleted file mode 100644 index a049da78be0f70..00000000000000 --- a/test/parallel/test-cluster-inspector-debug-port.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; -// Flags: --inspect={PORT} -const common = require('../common'); - -common.skipIfInspectorDisabled(); - -const assert = require('assert'); -const cluster = require('cluster'); -const debuggerPort = common.PORT; - -if (cluster.isMaster) { - function checkExitCode(code, signal) { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); - } - - function fork(offset, execArgv) { - if (execArgv) - cluster.setupMaster({execArgv}); - - const check = common.mustCall(checkExitCode); - cluster.fork({portSet: debuggerPort + offset}).on('exit', check); - } - - assert.strictEqual(process.debugPort, debuggerPort); - - fork(1); - fork(2, ['--inspect']); - fork(3, [`--inspect=${debuggerPort}`]); - fork(4, ['--inspect', `--debug-port=${debuggerPort}`]); - fork(5, [`--inspect-port=${debuggerPort}`]); - fork(6, ['--inspect', `--inspect-port=${debuggerPort}`]); -} else { - const hasDebugArg = process.execArgv.some(function(arg) { - return /inspect/.test(arg); - }); - - assert.strictEqual(hasDebugArg, true); - assert.strictEqual(process.debugPort, +process.env.portSet); - process.exit(); -} diff --git a/test/sequential/test-cluster-inspector-debug-port.js b/test/sequential/test-cluster-inspector-debug-port.js new file mode 100644 index 00000000000000..384f2b7a34b8fd --- /dev/null +++ b/test/sequential/test-cluster-inspector-debug-port.js @@ -0,0 +1,141 @@ +'use strict'; +// Flags: --inspect={PORT} +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const cluster = require('cluster'); +const debuggerPort = common.PORT; + +let offset = 0; + +function checkExitCode(code, signal) { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + +if (cluster.isMaster) { + assert.strictEqual(process.debugPort, debuggerPort); + + let message = 'If master is debugging with default port, ' + + 'forked worker should have debugPort, with offset = 1'; + + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = 'Debug port offset should increment'; + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = 'If master is debugging on non-default port, ' + + 'worker ports should be incremented from it'; + cluster.setupMaster({ + execArgv: ['--inspect=' + common.PORT] + }); + cluster.fork({ + portSet: common.PORT + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect=hostname:port argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect=localhost:' + (common.PORT + 10)] + }); + cluster.fork({ + portSet: common.PORT + 10 + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect=ipv4addr:port argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect=' + common.localhostIPv4 + ':' + (common.PORT + 20)] + }); + cluster.fork({ + portSet: common.PORT + 20 + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect=[ipv6addr]:port argument format ' + + 'should be parsed correctly'; + if (common.hasIPv6) { + cluster.setupMaster({ + execArgv: ['--inspect=[::1]:' + (common.PORT + 30)] + }); + cluster.fork({ + portSet: common.PORT + 30 + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + } + + message = '--inspect=hostname:port argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect=localhost'], + }); + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect=ipv4addr argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect=' + common.localhostIPv4] + }); + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect=[ipv6addr] argument format ' + + 'should be parsed correctly'; + if (common.hasIPv6) { + cluster.setupMaster({ + execArgv: ['--inspect=[::1]'] + }); + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + } + + message = '--inspect --debug-port={PORT} argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect', '--debug-port=' + debuggerPort] + }); + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + + message = '--inspect --inspect-port={PORT} argument format ' + + 'should be parsed correctly'; + cluster.setupMaster({ + execArgv: ['--inspect', '--inspect-port=' + debuggerPort] + }); + cluster.fork({ + portSet: process.debugPort + (++offset), + message + }).on('exit', common.mustCall(checkExitCode)); + +} else { + const hasDebugArg = process.execArgv.some(function(arg) { + return /inspect/.test(arg); + }); + + assert.strictEqual(hasDebugArg, true); + assert.strictEqual( + process.debugPort, + +process.env.portSet, + process.env.message + ); + process.exit(); +}