Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Large number of C++ objects unclaimed in node v0.12.7 #2813

Closed
gireeshpunathil opened this issue Sep 11, 2015 · 20 comments
Closed

Large number of C++ objects unclaimed in node v0.12.7 #2813

gireeshpunathil opened this issue Sep 11, 2015 · 20 comments
Labels
memory Issues and PRs related to the memory management or memory footprint. tls Issues and PRs related to the tls subsystem.

Comments

@gireeshpunathil
Copy link
Member

Node version: v0.12.7

Platform tested: Linux IA32

  1. Here is a standalone test case which reproduces the issue: https://github.com/gireeshpunathil/nodev0.12.7tlsmemoryleak
  2. Over time, Quite many objects of type WriteWrap, TLSWrap, SecureContext, TCP etc. pile up in the native heap, and the RSS growth drives node out of memory.
  3. Sample snapshots of the Object containment chain below. The leak is not apparent in the heapdump, as neither the comparison view nor the retention chain show up the anything, as these objects have lost their association with any JS objects.
  4. This issue seem related to Possible memory leak in TLS(Wrap?) #1075 but as the code base is different here, a fix does not look straightforward.
  5. We migrated from v0.10 to v0.12 due to an outstanding memory leak in the former. It would be great if this can be fixed in v0.12.7

image

image

image

@mscdex
Copy link
Contributor

mscdex commented Sep 11, 2015

Have you compared with node v4?

@mscdex mscdex added tls Issues and PRs related to the tls subsystem. memory Issues and PRs related to the memory management or memory footprint. labels Sep 11, 2015
@gireeshpunathil
Copy link
Member Author

@mscdex - thank you. I would like to test in v4, but: i) 'npm install heapdump' fails with that version of node, ii) I also see couple of outstanding memory leak issue in v4, iii) as I said earlier, we came from v0.10 to resolve some of the issues out there, so would like to work with v0.12.7, and hence this request.

bash-4.1$ npm install heapdump

heapdump@0.3.7 install /home/node/node_modules/heapdump
node-gyp rebuild

make: Entering directory /home/node/node_modules/heapdump/build' make: Warning: Fileaddon.target.mk' has modification time 53 s in the future
CXX(target) Release/obj.target/addon/src/heapdump.o
In file included from /home/.node-gyp/4.0.0/include/node/node.h:42,
from ../src/heapdump.cc:15:
/home/.node-gyp/4.0.0/include/node/v8.h:336: error: expected unqualified-id before âusingâ

@targos
Copy link
Member

targos commented Sep 11, 2015

@gireeshpunathil it is weird. I can install heapdump without error on v4. Maybe try to remove the node_modules directory and reinstall ?

@bnoordhuis
Copy link
Member

@gireeshpunathil You need a C++11-capable compiler, i.e., g++ 4.8 or clang++ 3.4 or newer.

@gireeshpunathil
Copy link
Member Author

@bnoordhuis thanks - I guessed something similar from the compiler error which warns about the C++ syntax.
I will try to fix this. Meanwhile, if the root cause of the leak in 0.12 can be detected, that would be great. Thanks once again!

@ChALkeR
Copy link
Member

ChALkeR commented Sep 11, 2015

Over time, Quite many objects of type WriteWrap, TLSWrap, SecureContext, TCP etc. pile up in the native heap, and the RSS growth drives node out of memory.

#1522, #1529, #1580 — fixed in v1.8.2.

@ChALkeR
Copy link
Member

ChALkeR commented Sep 13, 2015

@indutny Should this be backported to 0.12 branch or not?
If not, I guess we can close this issue.

@indutny
Copy link
Member

indutny commented Sep 13, 2015

@ChALkeR I think no.

@ChALkeR
Copy link
Member

ChALkeR commented Sep 18, 2015

Closing this. Feel free to reopen if this is going to be backported to 0.12 branch or if upgrading to 4.x doesn't fix things.

@ChALkeR ChALkeR closed this as completed Sep 18, 2015
@base698
Copy link

base698 commented Sep 21, 2015

We just upgraded to node 0.12.7. Our memory usage jumped from a baseline of about 350MB per node to 512MB per node--right at Heroku's max. It's fairly stable, just higher than normal. There are several issues mentioning a memory leak in later versions. What version is now safe? Should we jump to io.js 1.8.2 because the later memory leak is fixed?

@indutny
Copy link
Member

indutny commented Sep 21, 2015

@base698 I would go to 4.1.0 node.js

@base698
Copy link

base698 commented Sep 21, 2015

Is the amount of memory used higher? Found this stackoverflow.com post about the issue we are experiencing: http://stackoverflow.com/questions/29914837/node-js-0-12-x-memory-usage Said to use --max-old-space-size=512

@indutny
Copy link
Member

indutny commented Sep 21, 2015

@base698 I think the memory amount is pretty low now. cc @rvagg

@base698
Copy link

base698 commented Sep 22, 2015

We can't upgrade because of this issue effecting numerous packages: brianmcd/contextify#180 Seems @rvagg is referenced there as well.

@indutny
Copy link
Member

indutny commented Sep 22, 2015

Oh, sorry! I didn't read full thread. Thought, it was a rival of the old discussion.

@bnoordhuis
Copy link
Member

@base698 I don't know what you do with contextify but you may be able to drop the dependency when you switch to v4.x, the built-in vm module basically works the same as contextify now.

@base698
Copy link

base698 commented Sep 22, 2015

It's other modules we depend on--jsdom being one.
On Sep 22, 2015 3:19 AM, "Ben Noordhuis" notifications@github.com wrote:

@base698 https://github.com/base698 I don't know what you do with
contextify but you may be able to drop the dependency when you switch to
v4.x, the built-in vm module basically works the same as contextify now.


Reply to this email directly or view it on GitHub
#2813 (comment).

@bnoordhuis
Copy link
Member

What version of jsdom are you using? The v4.x releases only work with io.js v3.x and node.js v4.x.

@base698
Copy link

base698 commented Sep 22, 2015

Tried 3.5.0, 4.0.0 on you recommendation just now, and 6.5.0 was the
previous try.

npm install jsdom # fails with contextify C++ error mentioned here:
brianmcd/contextify#180 (comment)

On Tue, Sep 22, 2015 at 7:25 AM, Ben Noordhuis notifications@github.com
wrote:

What version of jsdom are you using? The v4.x releases only work with
io.js v3.x and node.js v4.x.


Reply to this email directly or view it on GitHub
#2813 (comment).

@base698
Copy link

base698 commented Sep 22, 2015

I did a small load test in our testing environment using node 12.7 and 4.1.0 for those interested. It does appear to have way better memory usage in 4.1.0.

At the start of the graph where the vertical black bar is node 4.1.0 was installed. I did a big load test and it looks to have GC'd and behaved fine. Memory flat lined at a pretty low value. We deployed node 0.12.7 at the second vertical black bar and mem usage immediately went high. Doing a small load test put the mem usage even higher than it got with node 4.1.0 during a large load test.

screen shot 2015-09-22 at 1 02 40 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory Issues and PRs related to the memory management or memory footprint. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants