Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

net: memory leak in net.connect #4308

Closed
bnoordhuis opened this Issue · 3 comments

1 participant

@bnoordhuis

The connect listener is added with .on(), not .once(), stopping it from getting garbage collected.

Here is a test case:

var common = { PORT: 5678 };
var assert = require('assert');
var net = require('net');

assert(typeof gc === 'function', 'Run this test with --expose-gc');
net.createServer(function() {}).listen(common.PORT);

(function() {
  // 2**26 == 64M entries
  for (var i = 0, junk = [123.456]; i < 26; ++i) junk = junk.concat(junk);

  if (process.argv[2] === 'once') {
    var conn = net.createConnection(common.PORT, '127.0.0.1');
    conn.once('connect', onConnect);
  }
  else {
    net.createConnection(common.PORT, '127.0.0.1', onConnect);  // uses on()
  }

  function onConnect() {
    assert(junk.length != 0);  // keep reference alive
    setTimeout(cleanup, 10);
    gc();
  }
})();

function cleanup() {
  var before = process.memoryUsage().rss;
  gc();
  var after = process.memoryUsage().rss;
  var reclaimed = (before - after) / 1024;
  console.log('%d kB reclaimed', reclaimed);
  assert(reclaimed > 384 * 1024);  // it's more like 512M on x64
  process.exit();
}

Using net.createConnection(cb):

$ out/Release/node --expose-gc tmp/test-net-connect-gc.js
-48 kB reclaimed

timers.js:102
            if (!process.listeners('uncaughtException').length) throw e;
                                                                      ^
AssertionError: false == true
    at cleanup [as _onTimeout] (/home/bnoordhuis/src/nodejs/master/tmp/test-net-connect-gc.js:33:3)
    at Timer.list.ontimeout (timers.js:100:19)

Using connection.once('connect', cb):

$ out/Release/node --expose-gc tmp/test-net-connect-gc.js once
524260 kB reclaimed

It's easy to fix but an audit of lib/ for variations of this bug might be in order.

@bnoordhuis bnoordhuis closed this issue from a commit
@bnoordhuis bnoordhuis net: fix net.connect() resource leak
The 'connect' event listener was attached with .on(), which blocked it from
getting garbage collected. Use .once() instead.

Fixes #4308.
4cb17cb
@bnoordhuis

Addressed in 4cb17cb.

@bnoordhuis bnoordhuis was assigned
@bnoordhuis

Reopening, this probably needs to be addressed separately in the tls module.

@bnoordhuis bnoordhuis reopened this
@bnoordhuis bnoordhuis closed this issue from a commit
@bnoordhuis bnoordhuis tls: fix tls.connect() resource leak
The 'secureConnect' event listener was attached with .on(), which blocked it
from getting garbage collected. Use .once() instead.

Fixes #4308.
121ed91
@bnoordhuis

Fixed for tls.connect() in 121ed91.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.