Commit b8213ba
cluster: ignore queryServer msgs on disconnection
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>1 parent 1bb2967 commit b8213ba
2 files changed
Lines changed: 50 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -445,6 +445,9 @@ function masterInit() { | |||
| 445 | 445 | } | |
| 446 | 446 | ||
| 447 | 447 | function queryServer(worker, message) { | |
| 448 | + // Stop processing if worker already disconnecting | ||
| 449 | + if (worker.suicide) | ||
| 450 | + return; | ||
| 448 | 451 | var args = [message.address, | |
| 449 | 452 | message.port, | |
| 450 | 453 | message.addressType, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,47 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + // Flags: --expose-internals | ||
| 3 | + | ||
| 4 | + const common = require('../common'); | ||
| 5 | + const assert = require('assert'); | ||
| 6 | + const net = require('net'); | ||
| 7 | + const cluster = require('cluster'); | ||
| 8 | + const handles = require('internal/cluster').handles; | ||
| 9 | + const os = require('os'); | ||
| 10 | + | ||
| 11 | + if (common.isWindows) { | ||
| 12 | + console.log('1..0 # Skipped: This test does not apply to Windows.'); | ||
| 13 | + return; | ||
| 14 | + } | ||
| 15 | + | ||
| 16 | + cluster.schedulingPolicy = cluster.SCHED_NONE; | ||
| 17 | + | ||
| 18 | + if (cluster.isMaster) { | ||
| 19 | + const cpus = os.cpus().length; | ||
| 20 | + const tries = cpus > 8 ? 128 : cpus * 16; | ||
| 21 | + | ||
| 22 | + const worker1 = cluster.fork(); | ||
| 23 | + worker1.on('message', common.mustCall(() => { | ||
| 24 | + worker1.disconnect(); | ||
| 25 | + for (let i = 0; i < tries; ++ i) { | ||
| 26 | + const w = cluster.fork(); | ||
| 27 | + w.on('online', common.mustCall(w.disconnect)); | ||
| 28 | + } | ||
| 29 | + })); | ||
| 30 | + | ||
| 31 | + cluster.on('exit', common.mustCall((worker, code) => { | ||
| 32 | + assert.strictEqual(code, 0, 'worker exited with error'); | ||
| 33 | + }, tries + 1)); | ||
| 34 | + | ||
| 35 | + process.on('exit', () => { | ||
| 36 | + assert.deepEqual(Object.keys(cluster.workers), []); | ||
| 37 | + assert.strictEqual(Object.keys(handles).length, 0); | ||
| 38 | + }); | ||
| 39 | + | ||
| 40 | + return; | ||
| 41 | + } | ||
| 42 | + | ||
| 43 | + var server = net.createServer(); | ||
| 44 | + | ||
| 45 | + server.listen(common.PORT, function() { | ||
| 46 | + process.send('listening'); | ||
| 47 | + }); | ||
0 commit comments