Memory leak with https request

[francois2metz]

Hi,

I have discovered a memory leak when create https request. Memory increased all the time.

It works fine with http request.

You can see the tescase: http://gist.github.com/493929

[sechrist]

This seems to be caused by something going wrong with the process of adding credentials in (if you didn't provide them). If I create credentials with crypto then reuse them for my HTTPS requests the leak doesn't exist.

http://github.com/ry/node/blob/master/src/node_crypto.cc#L191

http://i.imgur.com/XZa42.png

[ry]

updated test for v0.4.3

var https = require('https');

function request() {
    https.get({ host: 'google.com' });
}

var mem = null;

function report_stats() {
    var temp = process.memoryUsage();
    if (!mem) mem = temp;
    var diff = (temp.rss - mem.rss) / (1024*1024);
    console.log((temp.rss)/(1024*1024) +
                " Mb memory usage, diff "+ diff + " Mb");
    mem = temp;
}

setInterval(report_stats, 3*1000);
setInterval(request, 1000);

[grahamc]

+1 on this bug, I'm seeing similar results. (v0.4.8)

[bnoordhuis]

Is this still an issue with master? Ryan's test case shows RSS going up and down here. Tested with swapoff -a and all variations of mlockall() (to stop the kernel from discarding data and code pages).

[LiorCohen]

+1 with 0.4.9

[sh1mmer]

This isn't a memory leak. It's a GC issue. I modified the code @ry did to try this:

var https = require('https');

process.title = 'http-mem-test';

var count = 0;

function request() {
    https.get({ host: 'google.com' }, function() {
      if(count > 1000) {
        clearInterval(interval)
        //keep process open while GC happens
        var stdin = process.openStdin();
      } else {
        count += 1;
      }
    });
}

var mem = null;

function report_stats() {
    var temp = process.memoryUsage();
    if (!mem) mem = temp;
    var diff = (temp.rss - mem.rss) / (1024*1024);
    console.log((temp.rss)/(1024*1024) +
                " Mb memory usage, diff "+ diff + " Mb");
    mem = temp;
}

setInterval(report_stats, 3*1000);
var interval = setInterval(request, 1000);

It takes a while but v8 finally GCs down to less than the initial memory count from the first report_stats interval.

[francois2metz]

Not true in my case.

Started at 12.95703125 Mb at 11am. Stopped at 82.9765625 Mb at 7pm.

[sh1mmer]

Which test case were you using mine or Ry's? Are you using head?

Mine stops making requests after 1000 reqs to allow the GC to catch up. I think the point is that if you use a lot of https client requests you eat up memory faster than it can be GC'd. However it isn't actually leaking.

I'm not saying we shouldn't try to optimize but I can't see an actual leak.

[francois2metz]

With your testcase:

$> $ node -v
v0.5.9

$> time node memory.js
13.32421875 Mb memory usage, diff 0 Mb
15.890625 Mb memory usage, diff 2.56640625 Mb
16.234375 Mb memory usage, diff 0.34375 Mb
...
46.27734375 Mb memory usage, diff 0 Mb
46.28125 Mb memory usage, diff 0.00390625 Mb
46.21484375 Mb memory usage, diff -0.06640625 Mb
46.21484375 Mb memory usage, diff 0 Mb
46.21484375 Mb memory usage, diff 0 Mb
46.22265625 Mb memory usage, diff 0.0078125 Mb
46.2265625 Mb memory usage, diff 0.00390625 Mb

real    227m48.328s
user    0m16.561s
sys     0m1.580s

[sh1mmer]

This was a bit of a pain to figure out.

https://gist.github.com/1308628

Updated test case it takes almost an hour for v8 to clean up the memory. Maybe someone with a better understanding of the VM could shed some light on that.

[defunctzombie]

Is there a fix or workaround for this issue?

[trevnorris]

Just tested this @sh1mmer latest code against v0.8.15. Seemed to gc back to ~15MB memory. Though I did test against @ry's first test code, and there was a leak. After running for ~12 hours climbed from 15MB to 180MB. Feedback on whether this is still a bug?

[bnoordhuis]

Feedback on whether this is still a bug?

Undecided. I'm not able to reproduce it in a reasonable time frame (say 30 minutes).

The growing memory footprint you're seeing may be the result of V8 growing the heap in order to reduce mark-and-sweep cycles.

Do you get an OOM error when you cap the heap size with e.g. --max_old_space_size=75? What does the heap look like when you take a snapshot with https://github.com/bnoordhuis/node-heapdump ?

[trevnorris]

@bnoordhuis The size of Retained Memory in the profile is remaining constant, even though the memoryUsage().rss reports that it continues to grow.

Just to note: I ran the script with --max_old_space_size=35 but it reportedly grew to around 50MB in the time I allowed the script to run.

[bnoordhuis]

I ran the script with --max_old_space_size=35 but it reportedly grew to around 50MB in the time I allowed the script to run.

Yes, that's expected (or, at least, not unexpected) behavior. max_old_space_size controls the size of the V8 heap, it doesn't affect allocations made by node's binding layer (e.g. buffers).

I looked some more into it and I'm convinced there's no memory leak any more (assuming there ever was one). Here is a slightly modified test case:

var https = require('https');

function request() {
    https.get({ host: 'google.com' });
}

var mem = null;

var fs = require('fs');
function rss() {
  var pages = fs.readFileSync('/proc/self/stat', 'ascii').split(' ')[23];
  return { rss: 4096 * pages };
}

function report_stats() {
    if (typeof gc === 'function') gc();
    var temp = rss();
    if (!mem) mem = temp;
    var diff = (temp.rss - mem.rss) / (1024*1024);
    console.log((temp.rss)/(1024*1024) +
                " Mb memory usage, diff "+ diff + " Mb");
    mem = temp;
}

setInterval(report_stats, 3*1000);
setInterval(request, 1000);

Run it with node --expose-gc --trace-gc test.js. You'll find that after the first minute memory usage is rock stable.

strace -p $(pidof node) -e mmap,munmap,brk and watch 'pmap $(pidof node) | md5sum' will confirm that the memory footprint stops changing after a while.

The reason the test calls fs.readFileSync('/proc/self/stat') instead of process.memoryUsage() is that the latter calls fopen("/proc/self/stat") which in turns calls mmap().

mmap() modifies the process' page mappings and that changes the RSS (because RSS on modern operating system represents the section of virtual address space that's currently in use). One more reason why it's a bad indicator of memory leaks.