From 5d5b5fc31121500d8c787016e15da85f9a76cb5e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 24 Oct 2015 11:51:10 -0700 Subject: [PATCH] lib: fix cluster handle leak It is possible to cause a resource leak in SharedHandle. This commit fixes the leak. Fixes: https://github.com/nodejs/node/issues/2510 PR-URL: https://github.com/nodejs/node/pull/3510 Reviewed-By: Ben Noordhuis --- lib/cluster.js | 5 ++- test/parallel/test-cluster-shared-leak.js | 53 +++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-cluster-shared-leak.js diff --git a/lib/cluster.js b/lib/cluster.js index 602cc8d60b9200..eef8bd25639dd2 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -345,7 +345,10 @@ function masterInit() { * if it has disconnected, otherwise we might * still want to access it. */ - if (!worker.isConnected()) removeWorker(worker); + if (!worker.isConnected()) { + removeHandlesForWorker(worker); + removeWorker(worker); + } worker.suicide = !!worker.suicide; worker.state = 'dead'; diff --git a/test/parallel/test-cluster-shared-leak.js b/test/parallel/test-cluster-shared-leak.js new file mode 100644 index 00000000000000..b823989754b777 --- /dev/null +++ b/test/parallel/test-cluster-shared-leak.js @@ -0,0 +1,53 @@ +// In Node 4.2.1 on operating systems other than Linux, this test triggers an +// assertion in cluster.js. The assertion protects against memory leaks. +// https://github.com/nodejs/node/pull/3510 + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); +const cluster = require('cluster'); +cluster.schedulingPolicy = cluster.SCHED_NONE; + +if (cluster.isMaster) { + var conn, worker1, worker2; + + worker1 = cluster.fork(); + worker1.on('message', common.mustCall(function() { + worker2 = cluster.fork(); + conn = net.connect(common.PORT, common.mustCall(function() { + worker1.send('die'); + worker2.send('die'); + })); + // c.on('error', function(e) { + // // ECONNRESET is OK + // if (e.code !== 'ECONNRESET') + // throw e; + // }); + })); + + cluster.on('exit', function(worker, exitCode, signalCode) { + assert(worker === worker1 || worker === worker2); + assert.strictEqual(exitCode, 0); + assert.strictEqual(signalCode, null); + if (Object.keys(cluster.workers).length === 0) + conn.destroy(); + }); + + return; +} + +var server = net.createServer(function(c) { + c.end('bye'); +}); + +server.listen(common.PORT, function() { + process.send('listening'); +}); + +process.on('message', function(msg) { + if (msg !== 'die') return; + server.close(function() { + setImmediate(() => process.disconnect()); + }); +});