From d421e85dc933087c337c1f02ccb01ac28d25618b 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/5152 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- lib/cluster.js | 5 ++- test/simple/test-cluster-shared-leak.js | 48 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/simple/test-cluster-shared-leak.js diff --git a/lib/cluster.js b/lib/cluster.js index 71ebf41c454ee8..41c0e215ed5471 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -363,7 +363,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/simple/test-cluster-shared-leak.js b/test/simple/test-cluster-shared-leak.js new file mode 100644 index 00000000000000..add050133cd84f --- /dev/null +++ b/test/simple/test-cluster-shared-leak.js @@ -0,0 +1,48 @@ +// On some platforms this test triggers an assertion in cluster.js. +// The assertion protects against memory leaks. +// https://github.com/nodejs/node/pull/3510 + +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var net = require('net'); +var 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(); + worker2.on('online', function() { + conn = net.connect(common.PORT, common.mustCall(function() { + worker1.disconnect(); + worker2.disconnect(); + })); + conn.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'); +});