From cd23c8850b19468235459ea6ca968b75d0c46da2 Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Sun, 26 Apr 2015 02:01:35 +0300 Subject: [PATCH 1/6] cluster: sometimes debug-port value for child process can be broken In case of long running cluster, if worker processes is restarting periodically, --debug-port value may become out of range [1024, 65535]. --- lib/cluster.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cluster.js b/lib/cluster.js index 10a55996f15cc8..7c37a75445a067 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -17,6 +17,7 @@ cluster.Worker = Worker; cluster.isWorker = ('NODE_UNIQUE_ID' in process.env); cluster.isMaster = (cluster.isWorker === false); +const debugPortRange = (65536 - 1024); function Worker(options) { if (!(this instanceof Worker)) @@ -282,7 +283,7 @@ function masterInit() { function createWorkerProcess(id, env) { var workerEnv = util._extend({}, process.env); var execArgv = cluster.settings.execArgv.slice(); - var debugPort = process.debugPort + id; + var debugPort = (process.debugPort + id) % debugPortRange + 1024; var hasDebugArg = false; workerEnv = util._extend(workerEnv, env); From e71cfc893887c5a5139f5e04640091584c5255b8 Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Mon, 27 Apr 2015 11:20:59 +0300 Subject: [PATCH 2/6] Test added --- .../test-cluster-debugport-overflow.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/parallel/test-cluster-debugport-overflow.js diff --git a/test/parallel/test-cluster-debugport-overflow.js b/test/parallel/test-cluster-debugport-overflow.js new file mode 100644 index 00000000000000..3639243f71394e --- /dev/null +++ b/test/parallel/test-cluster-debugport-overflow.js @@ -0,0 +1,18 @@ +var spawn = require('child_process').spawn; +var cluster = require('cluster'); +var assert = require('assert'); + +if (process.argv[2] == 'master') { + if (cluster.isMaster) { + cluster.fork().on('exit', function(code) { + process.exit(code); + }) + } else { + process.exit(42); + } +} else { + // iojs --debug-port=65535 test-cluster-debugport-overflow.js master + spawn(process.argv[0], ['--debug-port=65535', __filename, 'master']).on('close', function(code){ + assert.equal(42, code, 'Worker was started'); + }); +} From 2a8c5e7923f15b262121bc135150386024c4841a Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Mon, 27 Apr 2015 12:56:37 +0300 Subject: [PATCH 3/6] Fixed coding style issues --- lib/cluster.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cluster.js b/lib/cluster.js index 7c37a75445a067..59020608883c52 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -17,7 +17,8 @@ cluster.Worker = Worker; cluster.isWorker = ('NODE_UNIQUE_ID' in process.env); cluster.isMaster = (cluster.isWorker === false); -const debugPortRange = (65536 - 1024); +const kDebugPortStart = 1024; +const kDebugPortRange = (65536 - kDebugPortStart); function Worker(options) { if (!(this instanceof Worker)) @@ -283,7 +284,7 @@ function masterInit() { function createWorkerProcess(id, env) { var workerEnv = util._extend({}, process.env); var execArgv = cluster.settings.execArgv.slice(); - var debugPort = (process.debugPort + id) % debugPortRange + 1024; + var debugPort = (process.debugPort + id) % kDebugPortRange + kDebugPortStart; var hasDebugArg = false; workerEnv = util._extend(workerEnv, env); From 5ee069ba1972931cb035071836a0db886c8b6ce1 Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Mon, 27 Apr 2015 13:36:18 +0300 Subject: [PATCH 4/6] Fixed coding style issues --- lib/cluster.js | 4 +++- test/parallel/test-cluster-debugport-overflow.js | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/cluster.js b/lib/cluster.js index 59020608883c52..44cfc484e04149 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -284,9 +284,11 @@ function masterInit() { function createWorkerProcess(id, env) { var workerEnv = util._extend({}, process.env); var execArgv = cluster.settings.execArgv.slice(); - var debugPort = (process.debugPort + id) % kDebugPortRange + kDebugPortStart; + var debugPort = process.debugPort + id; var hasDebugArg = false; + debugPort = debugPort % kDebugPortRange + kDebugPortStart; + workerEnv = util._extend(workerEnv, env); workerEnv.NODE_UNIQUE_ID = '' + id; diff --git a/test/parallel/test-cluster-debugport-overflow.js b/test/parallel/test-cluster-debugport-overflow.js index 3639243f71394e..9ceb035bc46014 100644 --- a/test/parallel/test-cluster-debugport-overflow.js +++ b/test/parallel/test-cluster-debugport-overflow.js @@ -12,7 +12,8 @@ if (process.argv[2] == 'master') { } } else { // iojs --debug-port=65535 test-cluster-debugport-overflow.js master - spawn(process.argv[0], ['--debug-port=65535', __filename, 'master']).on('close', function(code){ + var args = ['--debug-port=65535', __filename, 'master']; + spawn(process.argv[0], args).on('close', function(code) { assert.equal(42, code, 'Worker was started'); }); } From 4aa3f47c4fc10f736a3811e02d3564043e46ca56 Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Mon, 27 Apr 2015 22:25:50 +0300 Subject: [PATCH 5/6] Rewrite from scratch. Debug port numbers must be consecutive. --- lib/cluster.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/cluster.js b/lib/cluster.js index 44cfc484e04149..7462d00c695f98 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -17,8 +17,8 @@ cluster.Worker = Worker; cluster.isWorker = ('NODE_UNIQUE_ID' in process.env); cluster.isMaster = (cluster.isWorker === false); -const kDebugPortStart = 1024; -const kDebugPortRange = (65536 - kDebugPortStart); +const kDebugPortMin = 1024; +const kDebugPortMax = 65535; function Worker(options) { if (!(this instanceof Worker)) @@ -281,14 +281,22 @@ function masterInit() { cluster.emit('setup', settings); } + var nextDebugPort = process.debugPort; + + function getDebugPort() { + ++nextDebugPort; + if (nextDebugPort > kDebugPortMax) { + nextDebugPort = kDebugPortMin; + } + return nextDebugPort; + } + function createWorkerProcess(id, env) { var workerEnv = util._extend({}, process.env); var execArgv = cluster.settings.execArgv.slice(); - var debugPort = process.debugPort + id; + var debugPort = getDebugPort(); var hasDebugArg = false; - debugPort = debugPort % kDebugPortRange + kDebugPortStart; - workerEnv = util._extend(workerEnv, env); workerEnv.NODE_UNIQUE_ID = '' + id; From 220e3d3179c477c7159ac541b3250b27967281cb Mon Sep 17 00:00:00 2001 From: Oleg Elifantiev Date: Tue, 28 Apr 2015 00:35:09 +0300 Subject: [PATCH 6/6] Missing semicolon --- test/parallel/test-cluster-debugport-overflow.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-cluster-debugport-overflow.js b/test/parallel/test-cluster-debugport-overflow.js index 9ceb035bc46014..4c149440c6f0d2 100644 --- a/test/parallel/test-cluster-debugport-overflow.js +++ b/test/parallel/test-cluster-debugport-overflow.js @@ -6,7 +6,7 @@ if (process.argv[2] == 'master') { if (cluster.isMaster) { cluster.fork().on('exit', function(code) { process.exit(code); - }) + }); } else { process.exit(42); }