search

pierreinglebert / node-zopfli

Node bindings for Zopfli Compression Algorithm (zlib, gzip, deflate compatible)
MIT License
110 stars 39 forks source link

Doesn't free memory #71

Closed jscheid closed 8 years ago

jscheid commented 8 years ago

Hi, Zopfli docs say that out must be freed after use: https://github.com/google/zopfli/blob/6818a0859063b946094fb6f94732836404a0d89a/src/zopfli/zopfli.h#L82-L83

However, there do not appear to be any calls to free in https://github.com/pierreinglebert/node-zopfli/blob/master/src/zopfli-binding.cc

Am I missing something? This would be a pretty massive memory leak -- basically it would mean memory the size of the compressed output is leaked, plus some overhead because zopfli allocates blocks sized with a power of two.

On a somewhat related note, we seem to get spurious segfaults. I'm not entirely sure that they are caused by node-zopfli but I do wonder, has anybody ever run this through valgrind? Since valgrind reports memory leaks, I'm guessing no?

pierreinglebert commented 8 years ago

Hi,

That's because Nan:NewBuffer handles it https://github.com/nodejs/nan/blob/master/doc/buffers.md#nannewbuffer

I ran it through valgrind on the first versions (before using NAN), there may be some memory leaks but as long as I'm running it with gulp as a script, it never bothered enough.

jscheid commented 8 years ago

Hi, ah -- that's what I was missing. Makes sense, thanks for the clarification.