From 20d337896967ccf04a5b66b5ee3d2bd08562cc17 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Wed, 25 May 2016 23:14:41 +0200 Subject: [PATCH] cluster: reset handle index on close It allows reopening a server after it has been closed. Fixes: https://github.com/nodejs/node/issues/6693 PR-URL: https://github.com/nodejs/node/pull/6981 Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: Ron Korving Reviewed-By: James M Snell --- lib/cluster.js | 26 +++++++------ .../test-cluster-server-restart-none.js | 37 +++++++++++++++++++ .../test-cluster-server-restart-rr.js | 37 +++++++++++++++++++ 3 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-cluster-server-restart-none.js create mode 100644 test/parallel/test-cluster-server-restart-rr.js diff --git a/lib/cluster.js b/lib/cluster.js index 24e3fafb3d3818..8e8bdefadf03c7 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -555,18 +555,18 @@ function workerInit() { // obj is a net#Server or a dgram#Socket object. cluster._getServer = function(obj, options, cb) { - const key = [ options.address, - options.port, - options.addressType, - options.fd ].join(':'); - if (indexes[key] === undefined) - indexes[key] = 0; + const indexesKey = [ options.address, + options.port, + options.addressType, + options.fd ].join(':'); + if (indexes[indexesKey] === undefined) + indexes[indexesKey] = 0; else - indexes[key]++; + indexes[indexesKey]++; const message = util._extend({ act: 'queryServer', - index: indexes[key], + index: indexes[indexesKey], data: null }, options); @@ -576,9 +576,9 @@ function workerInit() { if (obj._setServerData) obj._setServerData(reply.data); if (handle) - shared(reply, handle, cb); // Shared listen socket. + shared(reply, handle, indexesKey, cb); // Shared listen socket. else - rr(reply, cb); // Round-robin. + rr(reply, indexesKey, cb); // Round-robin. }); obj.once('listening', function() { cluster.worker.state = 'listening'; @@ -590,7 +590,7 @@ function workerInit() { }; // Shared listen socket. - function shared(message, handle, cb) { + function shared(message, handle, indexesKey, cb) { var key = message.key; // Monkey-patch the close() method so we can keep track of when it's // closed. Avoids resource leaks when the handle is short-lived. @@ -598,6 +598,7 @@ function workerInit() { handle.close = function() { send({ act: 'close', key: key }); delete handles[key]; + delete indexes[indexesKey]; return close.apply(this, arguments); }; assert(handles[key] === undefined); @@ -606,7 +607,7 @@ function workerInit() { } // Round-robin. Master distributes handles across workers. - function rr(message, cb) { + function rr(message, indexesKey, cb) { if (message.errno) return cb(message.errno, null); @@ -627,6 +628,7 @@ function workerInit() { if (key === undefined) return; send({ act: 'close', key: key }); delete handles[key]; + delete indexes[indexesKey]; key = undefined; } diff --git a/test/parallel/test-cluster-server-restart-none.js b/test/parallel/test-cluster-server-restart-none.js new file mode 100644 index 00000000000000..dc44bafd66317b --- /dev/null +++ b/test/parallel/test-cluster-server-restart-none.js @@ -0,0 +1,37 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); + +cluster.schedulingPolicy = cluster.SCHED_NONE; + +if (cluster.isMaster) { + const worker1 = cluster.fork(); + worker1.on('listening', common.mustCall(() => { + const worker2 = cluster.fork(); + worker2.on('exit', (code, signal) => { + assert.strictEqual(code, 0, 'worker2 did not exit normally'); + assert.strictEqual(signal, null, 'worker2 did not exit normally'); + worker1.disconnect(); + }); + })); + + worker1.on('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0, 'worker1 did not exit normally'); + assert.strictEqual(signal, null, 'worker1 did not exit normally'); + })); +} else { + const net = require('net'); + const server = net.createServer(); + server.listen(common.PORT, common.mustCall(() => { + if (cluster.worker.id === 2) { + server.close(() => { + server.listen(common.PORT, common.mustCall(() => { + server.close(() => { + process.disconnect(); + }); + })); + }); + } + })); +} diff --git a/test/parallel/test-cluster-server-restart-rr.js b/test/parallel/test-cluster-server-restart-rr.js new file mode 100644 index 00000000000000..724a77b71a4fa9 --- /dev/null +++ b/test/parallel/test-cluster-server-restart-rr.js @@ -0,0 +1,37 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); + +cluster.schedulingPolicy = cluster.SCHED_RR; + +if (cluster.isMaster) { + const worker1 = cluster.fork(); + worker1.on('listening', common.mustCall(() => { + const worker2 = cluster.fork(); + worker2.on('exit', (code, signal) => { + assert.strictEqual(code, 0, 'worker2 did not exit normally'); + assert.strictEqual(signal, null, 'worker2 did not exit normally'); + worker1.disconnect(); + }); + })); + + worker1.on('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0, 'worker1 did not exit normally'); + assert.strictEqual(signal, null, 'worker1 did not exit normally'); + })); +} else { + const net = require('net'); + const server = net.createServer(); + server.listen(common.PORT, common.mustCall(() => { + if (cluster.worker.id === 2) { + server.close(() => { + server.listen(common.PORT, common.mustCall(() => { + server.close(() => { + process.disconnect(); + }); + })); + }); + } + })); +}