src: plug memory leaks

[bnoordhuis]

In a few places dynamic memory was passed to the Buffer::New() overload
that makes a copy of the input, not the one that takes ownership.

This commit is a band-aid to fix the memory leaks. Longer term, we
should look into using C++11 move semantics more effectively.

R=@indutny, refs #2308.

I think there is another memory leak in TLSWrap::OnReadSelf() but parallel/test-https-drain starts failing erratically when I fix it... le sigh.

CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/72/

[indutny]

Argh, @trevnorris !

[indutny]

Wait, this one takes the ownership of base. You should not really fix it.

[indutny]

@bnoordhuis are you sure that they are leaking? I thought ArrayBuffer just takes ownership of them now?

[bnoordhuis]

Buffer::New() makes a copy of the data, see here.

[indutny]

Oh gosh, I was looking at it yesterday too. Looks like I was looking at wrong overloaded function then. Shouldn't it be Buffer::Use now, then?

[bnoordhuis]

I thought Buffer::Use() had been phased out because nothing in src/ uses it anymore. @trevnorris?

[kzc]

Buffer::Use() has much better memory utilization running @silverwind's leak detect script than this branch using Buffer::New(env, base, nread, FreePointer)

Buffer::Use() after 2 minutes - doesn't exceeds 600 MB:

303 MB
304 MB
368 MB
468 MB
304 MB
304 MB
390 MB
491 MB
313 MB
313 MB
405 MB
499 MB
302 MB
310 MB
406 MB
507 MB
291 MB

Buffer::New(env, base, nread, FreePointer) after 2 minutes:

670 MB
770 MB
864 MB
965 MB
1066 MB
1167 MB
1268 MB
1369 MB
1470 MB
462 MB
464 MB
483 MB
579 MB
677 MB
777 MB
878 MB
979 MB
1080 MB
1181 MB
1282 MB
1383 MB
1484 MB
462 MB

There's no leak with Buffer::New(env, base, nread, FreePointer) but it has a much higher in-use memory profile. Buffer::Use() appears to be the better fix.

[bnoordhuis]

Switched to Buffer::Use(), PTAL.

[silverwind]

Gave it a test run. It's an improvement, but still not anywhere near 2.5.0's almost constant usage in my FormData testcase. Will test further.

[bnoordhuis]

CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/74/

[silverwind]

Fixed CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/76/

(The default parameters were failing because they were still pointing to the io.js repo)

[trevnorris]

The implementation of Buffer::New() is wrong (my bad). It should always take control of memory. Hence why there's now also Buffer::Copy().

It was an oversight on my part to leave the internal implementation of Buffer::Use(). Basically, the fix should be to make Buffer::New() take control of memory, i.e. use Buffer::Use().

[bnoordhuis]

PTAL, I added Buffer::Copy() and renamed Buffer::Use() to Buffer::New().

CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/83/

[silverwind]

This looks to fix the leak in my simple test (#2308 (comment)) completely, good job.

I still see leakage in the FormData test (https://gist.github.com/silverwind/54b3829142f93d1127bc), but I'd say it's a important first step to land this now.

[trevnorris]

Nice job. LGTM.

[indutny]

LGTM!