Skip to content

Commit

Permalink
cluster: ignore queryServer msgs on disconnection
Browse files Browse the repository at this point in the history
It avoids the creation of unnecessary handles. This issue is causing
intermitent failures in `test-cluster-disconnect-race` on `FreeBSD`
and `OS X`.

The problem is that the `worker2.disconnect` is being called on the
master before the `queryServer` is handled, causing the worker to
be deleted, then the Server handle is created afterwards. Later on,
when `removeWorker` is called from the `exit` handler, there are no
workers left, but one handle, thus the `AssertionError`.

Add a new `test/sequential/test-cluster-disconnect-leak` based on
`test-cluster-disconnect-race` that creates lots of workers and fails
consistently without this patch.

PR-URL: #4465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
santigimeno authored and Myles Borins committed Feb 15, 2016
1 parent 1bb2967 commit b8213ba
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ function masterInit() {
}

function queryServer(worker, message) {
// Stop processing if worker already disconnecting
if (worker.suicide)
return;
var args = [message.address,
message.port,
message.addressType,
Expand Down
47 changes: 47 additions & 0 deletions test/sequential/test-cluster-disconnect-leak.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';
// Flags: --expose-internals

const common = require('../common');
const assert = require('assert');
const net = require('net');
const cluster = require('cluster');
const handles = require('internal/cluster').handles;
const os = require('os');

if (common.isWindows) {
console.log('1..0 # Skipped: This test does not apply to Windows.');
return;
}

cluster.schedulingPolicy = cluster.SCHED_NONE;

if (cluster.isMaster) {
const cpus = os.cpus().length;
const tries = cpus > 8 ? 128 : cpus * 16;

const worker1 = cluster.fork();
worker1.on('message', common.mustCall(() => {
worker1.disconnect();
for (let i = 0; i < tries; ++ i) {
const w = cluster.fork();
w.on('online', common.mustCall(w.disconnect));
}
}));

cluster.on('exit', common.mustCall((worker, code) => {
assert.strictEqual(code, 0, 'worker exited with error');
}, tries + 1));

process.on('exit', () => {
assert.deepEqual(Object.keys(cluster.workers), []);
assert.strictEqual(Object.keys(handles).length, 0);
});

return;
}

var server = net.createServer();

server.listen(common.PORT, function() {
process.send('listening');
});

0 comments on commit b8213ba

Please sign in to comment.