Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixes #3179 http.js leak while using http.get/request #3181

Closed
wants to merge 2 commits into from

5 participants

@vvo

Made a small changes that could means big change for anyone using http.get/request (like @mikeal request)

We were previously using self.on('response') on ClientRequest, meaning that any ClientRequest would stay in memory even when response is emitted.

So I changed that with .once since we should not get more than one reponse for a particular ClientRequest right ?

I hope I'm not mistaking. Tell me.

In #3179 I have attached a test file and some insights on memory usage from various node versions.

@stefounet

Tested on v0.6.15 : no more memory leak and tests ok (make test).

@isaacs
Owner

.once is the correct behavior here, since response can only happen once.

@vvo Can you please sign the CLA so that I can take your patch? Thanks.

@vvo
vvo commented

Done,

I hope that the merge remote upstream did not break the pull request, I'm discovering complex git situations :)

@mikeal

this is good code. more of these please :)

@isaacs
Owner

Landed on v0.6 and master.

@isaacs isaacs closed this
@isaacs
Owner

Also, I echo @mikeal's sentiment. Always nice to squash a bug with a 2-char fix :)

@c4milo

nice work @vvo

@vvo
vvo commented

Thanks all, I was thrilled to commit to node.js and will definitely look for more bugs to fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2012
Commits on May 2, 2012
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  lib/http.js
View
2  lib/http.js
@@ -1126,7 +1126,7 @@ function ClientRequest(options, cb) {
var method = self.method = (options.method || 'GET').toUpperCase();
self.path = options.path || '/';
if (cb) {
- self.on('response', cb);
+ self.once('response', cb);
}
if (!Array.isArray(options.headers)) {
Something went wrong with that request. Please try again.