Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Memory leak in unzip() #2328

Closed
vflash opened this issue Dec 14, 2011 · 13 comments
Closed

Memory leak in unzip() #2328

vflash opened this issue Dec 14, 2011 · 13 comments

Comments

@vflash
Copy link

vflash commented Dec 14, 2011

есть подозрение что в "unzip()" течет память.

there is a suspicion that the "unzip()" flowing memory.

var zlib = require('zlib');

var request = HTTP.request({...});

request.on('response', function(response) {
    ...

    response.on('data', function() {
        ...
    });

    response.on('end', function() {
        ...
        zlib.unzip(BufferJoin(body), function(error, buffer) {
            ...
        });

    });
});

amd , node - v0.6.5

@bnoordhuis
Copy link
Member

Sorry, but that's not enough to go on. Can you post a (working, ready to run) test case? I'm closing the issue until further notice.

@vflash
Copy link
Author

vflash commented Dec 14, 2011

@bnoordhuis bnoordhuis reopened this Dec 14, 2011
@isaacs
Copy link

isaacs commented Dec 14, 2011

I'm not seeing any memory leak here. What data do you see that makes you think it is leaky?

You are definitely using a lot more memory than necessary in this script, however. Why not just create a zlib.Unzip() object, and then write every chunk into it, rather than caching the whole thing in memory?

@vflash
Copy link
Author

vflash commented Dec 14, 2011

без unzip стабильно ~30mb памяти
с unzip постепенно растет, до 100 и более. я дальше не смотрел.

упростил код.
https://github.com/vflash/node-unzip-bug/blob/master/unziptest2.js


without stable unzip ~ 30mb memory
unzip with gradually increasing, to 100 or more. I have not looked further.

simplified the code.
https://github.com/vflash/node-unzip-bug/blob/master/unziptest2.js

@isaacs
Copy link

isaacs commented Dec 15, 2011

Yeah, like I said: You're using a lot of memory, but it's not leaking any. Here's a log: https://gist.github.com/1480107

The memory usage is not spiraling out of control. It's rising to a stable level, and then staying there, as one would expect from what your code is doing.

Not a bug, working as designed. You may find this document helpful: http://nodejs.org/docs/latest/api/zlib.html#memory_Usage_Tuning

@isaacs isaacs closed this as completed Dec 15, 2011
@vflash
Copy link
Author

vflash commented Dec 15, 2011

{ rss: 11341824, heapTotal: 5388736, heapUsed: 2791104 }
{ rss: 12279808, heapTotal: 7813568, heapUsed: 2288432 }
{ rss: 12472320, heapTotal: 7813568, heapUsed: 2306216 }
{ rss: 12480512, heapTotal: 7813568, heapUsed: 2306248 }
{ rss: 12492800, heapTotal: 7813568, heapUsed: 2306312 }
{ rss: 12500992, heapTotal: 7813568, heapUsed: 2306344 }

...

{ rss: 572416000, heapTotal: 12007872, heapUsed: 2336984 }
{ rss: 572739584, heapTotal: 12007872, heapUsed: 2337016 }
{ rss: 572792832, heapTotal: 12007872, heapUsed: 2337048 }
{ rss: 572850176, heapTotal: 12007872, heapUsed: 2337080 }
{ rss: 573173760, heapTotal: 12007872, heapUsed: 2337112 }
{ rss: 573227008, heapTotal: 12007872, heapUsed: 2337144 }
{ rss: 573284352, heapTotal: 12007872, heapUsed: 2337176 }
{ rss: 573607936, heapTotal: 12007872, heapUsed: 2337208 }

normal?

@vflash
Copy link
Author

vflash commented Dec 15, 2011

added to the code

zlib.unzip(buff, nulfunc); // test unzip
//zzz.push(BufferJoin([buff])); // intentional leak
//var x = BufferJoin([buff]); // normal code

@vflash
Copy link
Author

vflash commented Dec 17, 2011

@isaacs то о чем ты говоришь это память ядра v8. смотри на RES, он все время растет.


@ isaacs what are you talking about a kernel memory v8. Look at the RES, it is growing all the time.

@isaacs
Copy link

isaacs commented Dec 17, 2011

RSS is the resident set size. That number never goes down unless there is memory pressure, because libc doesn't actually de-allocate memory unless necessary when free() is called, preferring instead to mark the pages as unused. This greatly increases performance, but has the unfortunate side effect of making RSS a bad indicator of memory usage, since it will almost never shrink.

What you should see in practice is that the RSS steadily climbs at first, then levels off, and only changes slightly. Here's a test: https://gist.github.com/1490782

And the results are 100% expected: http://static.izs.me/zleak-test.log

RSS rises steadily, until it hits the point where there is memory pressure, then levels off.

It's not accurate to say it "never goes down". It never goes down until it has to, because the memory is still tied to that process, even though it's been freed. If I watch the System Monitor on my mac while running this test, the "Inactive Memory" sits at around 2GB, which is almost the entire amount of RSS in use by the node process. When the node process ends, "Inactive Memory" drops down to 85.4MB.

Since the node process was exerting memory pressure on the other processes, most of them gave up their inactive memory. When the node process ends, the total inactive memory in use is thus very low.

If you're looking at RSS to find memory leaks, then you need to watch for steep cliffs. Steady climbs are expected, and healthy.

@isaacs
Copy link

isaacs commented Dec 17, 2011

1 more point:

The reason why a steep cliff would be bad is that it would indicate that the process was getting nuked by the operating system. Of course, in this case, that would never happen. You'd just have your system get really sluggish as it paged out to disk.

@vflash
Copy link
Author

vflash commented Dec 17, 2011

хорошо, тогда почему в этом примере RSS не растет?
well, then why in this example, the RSS does not grow?

function test() {
timmer = false;
var x = BufferJoin([buff]); // normal code
};

RES в 5Гб и более, это ненормально. В тесте специально присутствуют паузы чтобы была возможно высвободить память, но этого не происходит.
RES in 5Gb and more, this is not normal. The test specifically there was a pause that may free up memory, but this does not happen.

не освобождать память плохая практики.
not release the memory is bad practice.

@isaacs
Copy link

isaacs commented Dec 17, 2011

@vflash What you are seeing is normal behavior. The vmstat utility shows that it's not paging to disk, even when running for many minutes. The RSS does go down, but only when it reaches the point where there is memory pressure. Almost the entire amount of RSS is "unused memory", that is, memory which has been free()'d, but not yet reclaimed by the system. That is completely normal, and expected.

There are no memory leaks here. You are misunderstanding what RSS is. I'm sorry, I don't know how better to demonstrate it.

@vflash
Copy link
Author

vflash commented Dec 17, 2011

Печально.
It is sad.

Повышенное потребление памяти может указывать на ошибку в коде. Если в проекте, будет такой модуль. то тестировать код будет сложнее.
Increased memory consumption may indicate a bug in the code. If the project will be a such module. then test the code will be harder.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants