Skip to content

Commit

Permalink
cluster: do not unconditionally set --debug-port
Browse files Browse the repository at this point in the history
Currently, each cluster worker is assigned an ever increasing
--debug-port argument. A long running cluster application that
does not use the debugger can run into errors related to the
port range. This commit mitigates the problem by only setting
the debug port if the master is started with debug arguments, or
the user explicitly defines debug arguments for the worker. This
commit also adds a new debug port offset counter that is only
incremented when a worker is created that utilizes debugging.

Fixes: nodejs/node-v0.x-archive#8159
Refs: #1524
PR-URL: #1949
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Oleg Elifantiev <oleg@elifantiev.ru>
  • Loading branch information
cjihrig authored and rvagg committed Aug 1, 2015
1 parent aadcba2 commit 612d163
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
10 changes: 4 additions & 6 deletions lib/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,11 @@ function masterInit() {
cluster.emit('setup', settings);
}

var debugPortOffset = 1;

function createWorkerProcess(id, env) {
var workerEnv = util._extend({}, process.env);
var execArgv = cluster.settings.execArgv.slice();
var debugPort = process.debugPort + id;
var hasDebugArg = false;

workerEnv = util._extend(workerEnv, env);
workerEnv.NODE_UNIQUE_ID = '' + id;
Expand All @@ -291,14 +291,12 @@ function masterInit() {
var match = execArgv[i].match(/^(--debug|--debug-(brk|port))(=\d+)?$/);

if (match) {
let debugPort = process.debugPort + debugPortOffset;
++debugPortOffset;
execArgv[i] = match[1] + '=' + debugPort;
hasDebugArg = true;
}
}

if (!hasDebugArg)
execArgv = ['--debug-port=' + debugPort].concat(execArgv);

return fork(cluster.settings.exec, cluster.settings.args, {
env: workerEnv,
silent: cluster.settings.silent,
Expand Down
38 changes: 38 additions & 0 deletions test/parallel/test-cluster-debug-port.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');

if (cluster.isMaster) {
assert.strictEqual(process.execArgv.length, 0, 'run test with no args');

function checkExitCode(code, signal) {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
}

console.log('forked worker should not have --debug-port');
cluster.fork().on('exit', checkExitCode);

cluster.setupMaster({
execArgv: ['--debug-port=' + process.debugPort]
});

console.log('forked worker should have --debug-port, with offset = 1');
cluster.fork({
portSet: process.debugPort + 1
}).on('exit', checkExitCode);

console.log('forked worker should have --debug-port, with offset = 2');
cluster.fork({
portSet: process.debugPort + 2
}).on('exit', checkExitCode);
} else {
const hasDebugArg = process.execArgv.some(function(arg) {
return /debug/.test(arg);
});

assert.strictEqual(hasDebugArg, process.env.portSet !== undefined);
assert.strictEqual(process.debugPort, +process.env.portSet || 5858);
process.exit();
}

0 comments on commit 612d163

Please sign in to comment.