Skip to content

Commit

Permalink
cluster: fix error on worker disconnect/destroy
Browse files Browse the repository at this point in the history
Avoid sending multiple `exitedAfterDisconnect` messages when
concurrently calling `disconnect()` and/or `destroy()` from the worker
so `ERR_IPC_DISCONNECTED` errors are not generated.

Fixes: #32106

PR-URL: #32793
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
santigimeno authored and targos committed May 7, 2020
1 parent fd7db6a commit eba1772
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
11 changes: 9 additions & 2 deletions lib/internal/cluster/child.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
48 changes: 48 additions & 0 deletions test/parallel/test-cluster-concurrent-disconnect.js
Original file line number Diff line number Diff line change
@@ -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]();
});
}

0 comments on commit eba1772

Please sign in to comment.