Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP memory leak when aborting the initiating request when using a pipe #19198

Closed
scic opened this issue Mar 7, 2018 · 3 comments
Closed

TCP memory leak when aborting the initiating request when using a pipe #19198

scic opened this issue Mar 7, 2018 · 3 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@scic
Copy link

scic commented Mar 7, 2018

I have a server that processes a GET request from the browser by issuing a GET request to another server. I use a pipe to forward the response.

http.createServer(function (req, res) {
  var url = 'http://jsonplaceholder.typicode.com/photos';
  var connector = http.get(url, received => received.pipe(res));
  req.pipe(connector);
}).listen(9001);

The problem is if the browser aborts the request then tcp memory gets leaked. The mem field of the following output keeps getting bigger until the server is restarted or the limit in /proc/sys/net/ipv4/tcp_mem is reached.

cat /proc/net/sockstat 
sockets: used 9243
TCP: inuse 7591 orphan 0 tw 7659 alloc 7671 mem 504111

After a while this might lead to an TCP: out of memory exception in the syslog (Happend on a CentOS but not on a Ubuntu):
kernel: TCP: out of memory -- consider tuning tcp_mem

If the server is stopped then the memory is released without stopping the memory is never recovered.

I tried to provide a simple example in https://gist.github.com/scic/466db4687f62fa23c0bc059c31ae179a.

I expect abort events to be automatically propagated to the piped stream or at least that the piped stream is properly destroyed.
Or must this be manually cleaned up and how?

@addaleax
Copy link
Member

addaleax commented Mar 7, 2018

Just so I’m not misunderstanding, you’re basically describing the problem as in https://www.npmjs.com/package/pump#what-problem-does-it-solve, right?

I think in that case either using pump or waiting for #13506 (to get a built-in pump()) would be the way to go … my understanding is that we can’t change the behavior in pipe() for backwards compatibility reasons?

/cc @nodejs/streams

@addaleax addaleax added the stream Issues and PRs related to the stream subsystem. label Mar 7, 2018
@mcollina
Copy link
Member

mcollina commented Mar 7, 2018

Yes.

@scic
Copy link
Author

scic commented Mar 8, 2018

Yes it seems to be the problem pump solves. Although I could not get the two pump streams connected with pump only thus i propagated the close event manually like this:

http.createServer(function (req, res) {
  var connector = http.get('http://jsonplaceholder.typicode.com/photos', received => pump(received, res));
  req.on('close', () => connector.destroy());
  return pump(req, connector);
}).listen(9001);

Now the tcp memory stays low.

I would appreciate an integrated pump functionality in Node.js like #13506 provides. I think its a very common use case.

@scic scic closed this as completed Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants