Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible memory leak when calling uv_queue_work from an addon multiple times #3560

Closed
oferh opened this issue Oct 28, 2015 · 7 comments
Closed
Labels
invalid Issues and PRs that are invalid.

Comments

@oferh
Copy link

oferh commented Oct 28, 2015

I've created a minimal asynchronous addon based on NAN example async_pi_estimate and ran it in a loop. It appears that there is a memory leak when using it because the code itself doesn't allocate
dynamic memory but after ~15 sec it crashes with:

FATAL ERROR:CALL_AND_RETRY_LAST Allocation failed - process out of memory

Also tried running twice with 2K iterations the memory footprint is as follows:

This is the output after the first round:

==28951== LEAK SUMMARY:
==28951==    definitely lost: 288 bytes in 1 blocks
==28951==    indirectly lost: 0 bytes in 0 blocks
==28951==      possibly lost: 7,241 bytes in 51 blocks
==28951==    still reachable: 1,084,994 bytes in 222 blocks
==28951==         suppressed: 0 bytes in 0 blocks

And this is the output after the 2nd one:

==28966== LEAK SUMMARY:
==28966==    definitely lost: 288 bytes in 1 blocks
==28966==    indirectly lost: 0 bytes in 0 blocks
==28966==      possibly lost: 7,241 bytes in 51 blocks
==28966==    still reachable: 1,249,474 bytes in 238 blocks
==28966==         suppressed: 0 bytes in 0 blocks

The code is available in this gist
Node version: 0.12.7

@oferh
Copy link
Author

oferh commented Oct 28, 2015

Another report on the same issue without NAN http://stackoverflow.com/questions/33346385/how-can-i-use-uv-queue-work-multiple-times

@ChALkeR ChALkeR added the memory Issues and PRs related to the memory management or memory footprint. label Oct 28, 2015
@bnoordhuis
Copy link
Member

Why do you report it here and not at https://github.com/nodejs/nan?

@bnoordhuis
Copy link
Member

Also, why are you trying to start one billion asynchronous work items? Small wonder it runs out of memory.

because the code itself doesn't allocate dynamic memory

What do you think this is? From your gist:

  Callback *callback = new Callback(info[0].As<Function>());

  AsyncQueueWorker(new AsyncTestWorker(callback));

I'll close the issue, I don't see a bug here.

@bnoordhuis bnoordhuis added the invalid Issues and PRs that are invalid. label Oct 28, 2015
@kkoopa
Copy link

kkoopa commented Oct 28, 2015

It was here too: nodejs/nan#501 but there is no reason it would leak from NAN's pov. AsyncQueueWorker deletes the worker after it has been executed, which takes the Callback with it. As far as I could tell, the issue is just that one should not start a billion asynchronous work items.

@ChALkeR ChALkeR removed the memory Issues and PRs related to the memory management or memory footprint. label Oct 28, 2015
@oferh
Copy link
Author

oferh commented Oct 28, 2015

Thanks @kkoopa and @bnoordhuis, it was my mistake the loop keeps creating new AsyncQueueWorker (threads) without waiting for the previous workers to complete and this of course is the reason for the memory issue.

@bnoordhuis
Copy link
Member

No problem. I think you posted to the libuv mailing list earlier this week, didn't you? Maybe you can post a follow-up?

@karimcitoh
Copy link

@bnoordhuis I was the one who posted to the libuv mailing list last week. Already posted a follow-up. This is not a bug. As @kkoopa said, AsyncQueueWorker deletes the worker after it has been executed. Thanks for the help!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

5 participants