diff --git a/lib/cluster.js b/lib/cluster.js index 8356fad88a..c8bf658d5b 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -434,7 +434,7 @@ function masterInit() { else if (message.act === 'listening') listening(worker, message); else if (message.act === 'suicide') - worker.suicide = true; + suicide(worker, message); else if (message.act === 'close') close(worker, message); } @@ -445,6 +445,11 @@ function masterInit() { cluster.emit('online', worker); } + function suicide(worker, message) { + worker.suicide = true; + send(worker, { ack: message.seq }); + } + function queryServer(worker, message) { // Stop processing if worker already disconnecting if (worker.suicide) @@ -541,7 +546,7 @@ function workerInit() { if (message.act === 'newconn') onconnection(message, handle); else if (message.act === 'disconnect') - worker.disconnect(); + _disconnect.call(worker, true); } }; @@ -662,14 +667,36 @@ function workerInit() { } Worker.prototype.disconnect = function() { + _disconnect.call(this); + }; + + Worker.prototype.destroy = function() { + this.suicide = true; + if (!this.isConnected()) process.exit(0); + var exit = process.exit.bind(null, 0); + send({ act: 'suicide' }, () => process.disconnect()); + process.once('disconnect', exit); + }; + + function send(message, cb) { + sendHelper(process, message, null, cb); + } + + function _disconnect(masterInitiated) { this.suicide = true; let waitingCount = 1; function checkWaitingCount() { waitingCount--; if (waitingCount === 0) { - send({ act: 'suicide' }); - process.disconnect(); + // If disconnect is worker initiated, wait for ack to be sure suicide + // is properly set in the master, otherwise, if it's master initiated + // there's no need to send the suicide message + if (masterInitiated) { + process.disconnect(); + } else { + send({ act: 'suicide' }, () => process.disconnect()); + } } } @@ -681,19 +708,6 @@ function workerInit() { } checkWaitingCount(); - }; - - Worker.prototype.destroy = function() { - this.suicide = true; - if (!this.isConnected()) process.exit(0); - var exit = process.exit.bind(null, 0); - send({ act: 'suicide' }, exit); - process.once('disconnect', exit); - process.disconnect(); - }; - - function send(message, cb) { - sendHelper(process, message, null, cb); } } diff --git a/test/parallel/test-regress-GH-3238.js b/test/parallel/test-regress-GH-3238.js index a92a09db5f..e6fe030bda 100644 --- a/test/parallel/test-regress-GH-3238.js +++ b/test/parallel/test-regress-GH-3238.js @@ -4,18 +4,19 @@ const assert = require('assert'); const cluster = require('cluster'); if (cluster.isMaster) { - const worker = cluster.fork(); - let disconnected = false; + function forkWorker(action) { + const worker = cluster.fork({ action }); + worker.on('disconnect', common.mustCall(() => { + assert.strictEqual(worker.suicide, true); + })); - worker.on('disconnect', common.mustCall(function() { - assert.strictEqual(worker.suicide, true); - disconnected = true; - })); + worker.on('exit', common.mustCall(() => { + assert.strictEqual(worker.suicide, true); + })); + } - worker.on('exit', common.mustCall(function() { - assert.strictEqual(worker.suicide, true); - assert.strictEqual(disconnected, true); - })); + forkWorker('disconnect'); + forkWorker('kill'); } else { - cluster.worker.disconnect(); + cluster.worker[process.env.action](); } diff --git a/test/sequential/test-cluster-disconnect-suicide-race.js b/test/sequential/test-cluster-disconnect-suicide-race.js new file mode 100644 index 0000000000..e05c420e1f --- /dev/null +++ b/test/sequential/test-cluster-disconnect-suicide-race.js @@ -0,0 +1,32 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const os = require('os'); + +if (cluster.isMaster) { + function forkWorker(action) { + const worker = cluster.fork({ action }); + worker.on('disconnect', common.mustCall(() => { + assert.strictEqual(worker.suicide, true); + })); + + worker.on('exit', common.mustCall(() => { + assert.strictEqual(worker.suicide, true); + })); + } + + const cpus = os.cpus().length; + const tries = cpus > 8 ? 64 : cpus * 8; + + cluster.on('exit', common.mustCall((worker, code) => { + assert.strictEqual(code, 0, 'worker exited with error'); + }, tries * 2)); + + for (let i = 0; i < tries; ++i) { + forkWorker('disconnect'); + forkWorker('kill'); + } +} else { + cluster.worker[process.env.action](); +}