nodejs / node-v0.x-archive

Moved to https://github.com/nodejs/node
34.42k stars 7.31k forks source link

net leaking received buffers #3371

Closed Jimbly closed 12 years ago

Jimbly commented 12 years ago

On Linux node 0.7.9, even under non-extreme load, process memory usage grows linearly with received data count, so it seems those Buffers are never getting garbage collected anymore.

An echo server plus client sending lots of data demonstrates this: https://gist.github.com/2870997

Note: that echo server also demonstrates some other bugs referenced here: https://groups.google.com/group/nodejs/browse_thread/thread/7a044cc3c8c3eef2 , but for the sake of this specific bug, note that on node 0.7.9 the client's RSS steadily climbs, but does not on 0.6.18. The server's RSS continuously climbs as long as there is load, unrelated to this specific bug.

bnoordhuis commented 12 years ago

Thanks, reproduced. Will look into it.

bnoordhuis commented 12 years ago

Interesting... it turns out to be an event loop starvation problem, at least for the server.

libuv tries to drain the kernel receive buffer but the client keeps filling it up faster than the server can consume it, making it loop indefinitely in uv__read(). That's trivially solved by putting an upper limit on the number of consecutive reads.

Now to find out what makes client memory usage explode...

Jimbly commented 12 years ago

Ah, great that you found the server issue (that's been around for a long time), and I was going to start a different bug about that. That description definitely matches the behavior I was seeing (I think setInterval/setTimeout etc was also not being called).

The client issue appears to be new in node 0.7 though, shouldn't be any starvation there, just leaking under simple circumstances, I think.

shigeki commented 12 years ago

Memory leaks would be occurred in slab_allocator because persistent handles were not disposed because they had already cleared just before. Fixed in 98ce4c69b4a215c8a45f8c6f6b45c4e06e2b600c butI did not write a test code because current master is too unstable to test. This patch seems to be working fine in my environment, please try it if the leak solved. The result output with patched in 0.7.9 is shown below.

unixjp:~/tmp/leak> ~/tmp/oldnode/node-v0.7.9/node client.js
connected.
2000 pkts, 1988.1 pkts/s, 20 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 7.8 MB sent, 7.7 MB received, 15.4 MB RSS
4100 pkts, 2000.0 pkts/s, 20 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 16.0 MB sent, 15.9 MB received, 13.7 MB RSS
6200 pkts, 1998.1 pkts/s, 20 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 24.2 MB sent, 24.1 MB received, 22.0 MB RSS
6660 pkts, 1974.2 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 26.0 MB sent, 26.0 MB received, 24.2 MB RSS
8660 pkts, 1998.0 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 33.8 MB sent, 33.8 MB received, 32.3 MB RSS
10680 pkts, 2010.0 pkts/s, 0 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 41.7 MB sent, 41.6 MB received, 10.4 MB RSS
12700 pkts, 1998.0 pkts/s, 20 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 49.6 MB sent, 49.5 MB received, 18.4 MB RSS
13320 pkts, 1955.8 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 52.0 MB sent, 52.0 MB received, 20.8 MB RSS
15340 pkts, 2016.0 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 59.9 MB sent, 59.8 MB received, 28.7 MB RSS
17340 pkts, 1998.0 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 67.7 MB sent, 67.6 MB received, 36.6 MB RSS
19360 pkts, 2000.0 pkts/s, 0 waiting for cb, 0.1 MB un-echo'd, 0 buffered, 75.6 MB sent, 75.5 MB received, 13.7 MB RSS
20000 pkts, 2000.0 pkts/s, 0 waiting for cb, 0.0 MB un-echo'd, 0 buffered, 78.0 MB sent, 78.0 MB received, 16.0 MB RSS
bnoordhuis commented 12 years ago

Thanks, landed in 208d171. Between that commit and 0a2076b2, the memory leaks and loop starvation issues seem to have been solved.

shigeki commented 12 years ago

@bnoordhuis Thanks. Let's make a brand-new and evolving libuv and node be more stable for the forthcoming new release.

bnoordhuis commented 12 years ago

Sorry Shigeki, no can do. If I make Node too stable, I'll be out of a job. :-)

shigeki commented 12 years ago

Don't worry. You never loose your job unless Node stops evolving :-) Recent changes on libuv are noted as a new great speciation in the evolutionary history of Node.