node-http-proxy leaks file descriptors

[jwalton]

Run this:

http = require('http');
httpProxy = require('http-proxy');

proxy = httpProxy.createProxyServer();
server = http.createServer(function(req, res) {
    return proxy.web(req, res, {
      target: 'http://example.com'
    });
});

server.listen(9000);

• ps -ef | grep node to figure out the PID that's running it.
• watch 'lsof -p 5966 | wc -l' (replace 5966 with your pid)
• Point a browser at http://localhost:9000 (you'll get a 404 from example.com). I was using Chrome and FF.
• Hit CTRL-R over and over.
• Watch your open file count climb.

Sockets don't seem to close for 2 minutes. On OS/X, heavy traffic will run out of file descriptors very quickly (my product's Selenium test case kills node-http-proxy about 1/10th of the way through.)

Using node v0.10.25 and node-http-proxy 1.0.2.

[jwalton]

Note this doesn't seem to happen in 1.0.1.

[jcrugzz]

@jwalton wow really? the changes between 1.0.1 and 1.0.2 don't touch the web proxy implementation. Are you positive this doesn't happen with 1.0.1? Ill do some testing myself

[jwalton]

Er... This exact scenario doesn't happen in 1.0.1. But, in my non-trivial version of this, where I also use webservice proxying, I see this number climb. My Selenium test suite still fails going through 1.0.1. :P

[jwalton]

Yeah, I'm just looking at the 1.0.1 vs 1.0.2 compare view, and there's nothing that could account for this. :/

[jwalton]

My coworker @goffrie discovered that this seems to fix things:

http = require('http');
httpProxy = require('http-proxy');

proxy = httpProxy.createProxyServer();
server = http.createServer(function(req, res) {
  req.headers.connection = "Close";
  return proxy.web(req, res, {
    target: 'http://example.com'
  });
});

server.listen(9000);

Setting connection = 'Close' means the back-end server is going to close the connection right after we make a request, which suggests this problem has to do with not reusing backend connections.

[goffrie]

Yeah - so it looks like the proxy server ends up making a ton of keep-alive connections to the backend server, each of which is used for only one request (and then kept alive for 2 minutes).

[Rush]

Does it happen with node 0.11.10 as well?

[glasser]

Isn't this #488?

Last I checked, if you don't set an agent: explicitly when using http-proxy 1.x, it will default to agent: false and leak.

I apologize for not having found the time to improve my original PR. We worked around this in my project by always using an explicit agent (which made sense for us for other reasons), but this leaves the default behavior as broken.

[jcrugzz]

I honestly think this is http being broken in 0.10.x without using an agent. This is one of the reasons http was a big focus for 0.12.x. I would either use an agent with a larger amount of sockets (since default is 5 and thats why this defaults to false) or use that workaround until it is fixed in 0.12.x. I believe in #488 this was confirmed to be a bug in node core that was fixed in 0.11.x. Closing the connection when we do not have an agent would not be the correct default behavior either so we just can't win in this situation currently.

[jwalton]

You are correct, sir. This does seem to work on 0.11.x, and this:

proxy = httpProxy.createProxyServer({
    agent: new http.Agent()
});

fixes the example above.

[jwalton]

Copying this here from PR #572: node-http-proxy is calling http.request() to proxy connections. In node 0.10.25, this creates a new ClientRequest. In node 0.11.11, http.request calls into globalAgent.request(). If you pass in false as the agent, then globalAgent.request() will modify your options and assign you a new agent for each request, however the new agent will turn off keep-alive. That's why it works in 0.11.

[sebjameswml]

I've fun into this issue myself, using node 0.10.25 and http-proxy 1.0.2. I'm using the workaround in which one sets request.headers.connection to "Close", rather than setting an agent for the proxy. Is that the best workaround for now?

[Reggino]

Running into same issue here. And fix/workaround confirmed: setting the agent makes the issue disappear! Thanks.

[simoami]

Just wanted to confirm that the globalAgent fix works here as well.

In my case I used:

targetObj.agent = targetObj.protocol === 'https:' ? https.globalAgent : http.globalAgent;

[jcrugzz]

fixed in 1.0.3