HTTPParser inhibits garbage collection on keep-alive connections

[jbellenger]

• Version: v7.0.0
• Platform: Darwin hostname redacted 15.6.0 Darwin Kernel Version 15.6.0: Thu Sep 1 15:01:16 PDT 2016; root:xnu-3248.60.11~2/RELEASE_X86_64 x86_64

I'm not too familiar with node internals, so please correct any details that I may have gotten wrong.

HTTPParser objects appear to be created on a per-socket basis, and retain an incoming reference to an IncomingMessage. The incoming reference is kept around even after the message has been completely parsed and handled, making it un-gc-able for as long as the underlying socket remains open.

At Twitter, we've found that this can contribute to high memory usage when:

• node is running behind a large pool of proxies with keep-alive connections to our service
• using express locals to store a large chunk of application state on the request.

In this environment, already-handled messages are unable to be garbage collected until either a new request comes in on the existing socket or the connection is closed.

I put together a small demo of this issue at jbellenger/node-message-retention

[addaleax]

@nodejs/http

[sam-github]

Duplicate of #9268, PR #9440

[targos]

#9440 landed.

[misterdjules]

It seems that the original comment and repro code were describing a memory leak on the server side, not on the client side. However, it seems that #9440 fixes a leak in the HTTP client's implementation.

I would think that this issue is not a duplicate of #9628, and is not fixed by #9440. As a result, reopening it, my apologies if I'm missing something.

[iagomelanias]

I don't know if it can be related, but i'm posting here.

We have tested several http libraries (request, superagent, got, node-fetch) pointing to a specific url. After running about ~20k requests in some minutes, we got this in our heapdump comparison:

HTTPPARSER seems to be allocating more and more memory, but it's not being released after we stop sending traffic. 10 minutes after stopping to send traffic, we still were using about ~250mb per cluster while we generally use about ~80mb without using any request library.

We reloaded our app some times to do not strike the peak.

While we keep sending traffic and using a request library, the memory keeps growing and growing until the app crashes. As the url we're using has a keepalive connection, this may be related.

[Trott]

Should this remain open?

[bnoordhuis]

I think this is still relevant. I'll take a look.

[bnoordhuis]

I can confirm that it's still an issue but I suspect it cannot be fixed.

The request object is stored in the state.incoming array and bound to the resOnFinish() callback in order to clean up outstanding request body data when the response is finalized or aborted.

What is wryly amusing is that it's a mitigation for resource leaks when people hold on to the request object but it evidently replaces one kind of leak with another.

[mflash1]

Node V8.2.1
we stream files using multer by sending http multipart requests.
we see the same memory leak on the server , HTTPParser objects are created per request and are not released after upload is ended. each one of them consumes
the memory consumption increasing and GC is unable to release the objects.

[ischyron]

Node v6.10.3

@iagomelanias node-fetch does not support keep-alive. https://github.com/bitinn/node-fetch#class-request.

Wondering if this happens to even non keep-alive requets ?

I see that http-parser is allocated even after load stops and after manually triggering gc on a SIGINT.