From e05c29db3fdacdf843cae43fd9e96ba566c9ca87 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Wed, 1 Apr 2020 14:22:07 +0200 Subject: [PATCH] cluster: fix error on worker disconnect/destroy Avoid sending multiple `exitedAfterDisconnect` messages when concurrently calling `disconnect()` and/or `destroy()` from the worker so `ERR_IPC_DISCONNECTED` errors are not generated. Fixes: https://github.com/nodejs/node/issues/32106 PR-URL: https://github.com/nodejs/node/pull/32793 Reviewed-By: Zeyu Yang Reviewed-By: Anna Henningsen --- lib/internal/cluster/child.js | 11 ++++- .../test-cluster-concurrent-disconnect.js | 48 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-cluster-concurrent-disconnect.js diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js index 250a82ecabaa34..74f30c0d2ece90 100644 --- a/lib/internal/cluster/child.js +++ b/lib/internal/cluster/child.js @@ -228,16 +228,23 @@ function _disconnect(masterInitiated) { // Extend generic Worker with methods specific to worker processes. Worker.prototype.disconnect = function() { - _disconnect.call(this); + if (![ 'disconnecting', 'destroying' ].includes(this.state)) { + this.state = 'disconnecting'; + _disconnect.call(this); + } + return this; }; Worker.prototype.destroy = function() { - this.exitedAfterDisconnect = true; + if (this.state === 'destroying') + return; + this.exitedAfterDisconnect = true; if (!this.isConnected()) { process.exit(0); } else { + this.state = 'destroying'; send({ act: 'exitedAfterDisconnect' }, () => process.disconnect()); process.once('disconnect', () => process.exit(0)); } diff --git a/test/parallel/test-cluster-concurrent-disconnect.js b/test/parallel/test-cluster-concurrent-disconnect.js new file mode 100644 index 00000000000000..22f72040263027 --- /dev/null +++ b/test/parallel/test-cluster-concurrent-disconnect.js @@ -0,0 +1,48 @@ +'use strict'; + +// Ref: https://github.com/nodejs/node/issues/32106 + +const common = require('../common'); + +const assert = require('assert'); +const cluster = require('cluster'); +const os = require('os'); + +if (cluster.isMaster) { + const workers = []; + const numCPUs = os.cpus().length; + let waitOnline = numCPUs; + for (let i = 0; i < numCPUs; i++) { + const worker = cluster.fork(); + workers[i] = worker; + worker.once('online', common.mustCall(() => { + if (--waitOnline === 0) + for (const worker of workers) + if (worker.isConnected()) + worker.send(i % 2 ? 'disconnect' : 'destroy'); + })); + + // These errors can occur due to the nature of the test, we might be trying + // to send messages when the worker is disconnecting. + worker.on('error', (err) => { + assert.strictEqual(err.syscall, 'write'); + assert.strictEqual(err.code, 'EPIPE'); + }); + + worker.once('disconnect', common.mustCall(() => { + for (const worker of workers) + if (worker.isConnected()) + worker.send('disconnect'); + })); + + worker.once('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + })); + } +} else { + process.on('message', (msg) => { + if (cluster.worker.isConnected()) + cluster.worker[msg](); + }); +}