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

Memory leak in Node.js #1417

Closed
bounc3-paradise-on-e opened this issue Aug 7, 2018 · 10 comments
Closed

Memory leak in Node.js #1417

bounc3-paradise-on-e opened this issue Aug 7, 2018 · 10 comments

Comments

@bounc3-paradise-on-e
Copy link

  • Node.js Version: v10.7.0
  • OS: Windows XP, 7, 8, 8.1, 10 all x64
  • Scope: code
  • Module: node-gyp
var myModule = require('../build/release/mymodule.node');
global.b = 123;

(function(){
  var a = 5;

  myModule();

  console.log(a, b, global.b); // Should print: 7, 8, 123
})();

myModule is a C++ module that is a function, which should be able to access local scope of the function from where it is called (like eval) and modify value of a (set it to 7) and also create local (not global) variable b with value 8. How to do that?

Calling myModule should be equivalent of calling eval('a=7;var b=8'). Basically, is there a way to access parent scope variable list and modify it directly from v8?

Many thx.

@gireeshpunathil
Copy link
Member

from language (JS / C++) perspective, the locals remain locals to a method, so there is no semantics that allow it.

from system's perspective, a hack is possible: callee being C++, it could unwind a couple of stack frames, locate the caller's function object and modify the locals before it returns - but it is a pure hack.

from v8's perspective, I don't know, but probably @nodejs/v8 would.

@hashseed
Copy link
Member

hashseed commented Aug 8, 2018

There is no easy way to do this. Locals that are not bound are stack-allocated. You could use the inspector to modify it, but the inspector doesn't support modifying locals if the frame is of an optimized function.

Could you describe your use case? Maybe there is a better way?

@bounc3-paradise-on-e
Copy link
Author

Thanks

My use case is like this: Suppose I have following script

for(;;){
  new class{};

  // Optional (requires --expose-gc flag)
  gc();
}

I run it and then monitor process memory allocations using some memory monitor. I see that memory is growing and after some time (leave it for several hours) it crashes the Node.js process.

I googled a bit and still not sure why it happens. This is simplified example, but my real app should run in a synchronous loop and synchronously communicate with other apps, but this crashing behavior is problematic.

So I tried to create a workaround: a C++ module that cleans up parent scope stack and deallocate everything that gc is unable to deallocate.

Maybe there is a better way?

That would be great, and also I would like to know why this code causes node.js to crash, I mean what is preventing the gc from doing what it is supposed to do? I also noticed if the infinite for loop is replaced with setInterval, then memory is not growing uncontrollable and stays at about 10MB. But in a for loop it grows and crashes.

@hashseed
Copy link
Member

hashseed commented Aug 8, 2018

I see. So you are actually dealing with a memory leak.

Do you still observe the same memory increase if you refactor the loop body into a new function, and call it from the for-loop? Also make sure to use let-declarations instead of var-declarations.

@bounc3-paradise-on-e
Copy link
Author

bounc3-paradise-on-e commented Aug 8, 2018

With this code memory increases at speed ~24KB/s.
It will reach 1.5GB default limit within 18 hours.

'use strict';

let func = () => {
  new class{};
  gc(); // Just to reduce memory fluctuating
};

for(;;){
  func();
}

I'm starting to believe that this is some sort of v8 (or node) bug. I run this code on Node.js v4.0.0 and memory doesn't increase. Memory increases starting from version v6.0.0 I think.

And even if using var instead of let and even if not using separate function (which seems to have no effect anyway), no leak should occur, right? I mean, there are no permanent references involved. A new class is defined, then instantiated and the object should be GCed asap.

Tested on gecko repl and memory is not increasing at all. What may be the problem here, if it is not a v8 regression (especially according to the fact that this doesn't leak in node v4.0.0)?

@hashseed
Copy link
Member

hashseed commented Aug 8, 2018

Indeed seems like a V8 bug. I filed an issue: https://bugs.chromium.org/p/v8/issues/detail?id=8037

@hashseed
Copy link
Member

hashseed commented Aug 8, 2018

Actually, I can't reproduce this:

yangguo@yangguo2:~/node$ cat test.js
'use strict';

for(var i =0; i< 1E7;i++) {
  if (i % 1E4 == 0) gc();
  new class{};
}
yangguo@yangguo2:~/node$ ./node --trace-gc --expose-gc test.js | head -n20
[227134:0x5560ceb22180]       55 ms: Scavenge 2.1 (4.2) -> 1.9 (5.2) MB, 1.6 / 0.0 ms  (average mu = 1.000, current mu = 1.000) allocation failure 
[227134:0x5560ceb22180]       68 ms: Scavenge 2.3 (5.2) -> 2.2 (6.2) MB, 1.8 / 0.0 ms  (average mu = 1.000, current mu = 1.000) allocation failure 
[227134:0x5560ceb22180]       99 ms: Mark-sweep 3.0 (6.2) -> 2.6 (8.2) MB, 3.4 / 0.0 ms  (average mu = 1.000, current mu = 1.000) testing GC in old space requested
[227134:0x5560ceb22180]      106 ms: Scavenge 5.2 (9.2) -> 4.9 (10.2) MB, 1.7 / 0.0 ms  (average mu = 1.000, current mu = 1.000) allocation failure 
[227134:0x5560ceb22180]      110 ms: Scavenge 5.9 (10.7) -> 5.8 (16.2) MB, 1.7 / 0.0 ms  (average mu = 1.000, current mu = 1.000) allocation failure 
[227134:0x5560ceb22180]      123 ms: Mark-sweep 10.9 (19.2) -> 2.8 (15.7) MB, 2.5 / 0.0 ms  (average mu = 0.895, current mu = 0.895) testing GC in old space requested
[227134:0x5560ceb22180]      135 ms: Scavenge 9.1 (17.7) -> 8.5 (17.7) MB, 1.9 / 0.0 ms  (average mu = 0.895, current mu = 0.895) allocation failure 
[227134:0x5560ceb22180]      139 ms: Scavenge 9.5 (17.7) -> 9.2 (29.2) MB, 2.7 / 0.0 ms  (average mu = 0.895, current mu = 0.895) allocation failure 
[227134:0x5560ceb22180]      144 ms: Mark-sweep 10.2 (29.7) -> 2.8 (24.2) MB, 2.8 / 0.0 ms  (average mu = 0.881, current mu = 0.865) testing GC in old space requested
[227134:0x5560ceb22180]      159 ms: Mark-sweep 11.0 (26.7) -> 2.7 (22.7) MB, 2.7 / 0.0 ms  (average mu = 0.855, current mu = 0.817) testing GC in old space requested
[227134:0x5560ceb22180]      173 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.2 / 0.0 ms  (average mu = 0.851, current mu = 0.846) testing GC in old space requested
[227134:0x5560ceb22180]      187 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.2 / 0.0 ms  (average mu = 0.846, current mu = 0.840) testing GC in old space requested
[227134:0x5560ceb22180]      201 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.2 / 0.0 ms  (average mu = 0.843, current mu = 0.839) testing GC in old space requested
[227134:0x5560ceb22180]      215 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.2 / 0.0 ms  (average mu = 0.842, current mu = 0.840) testing GC in old space requested
[227134:0x5560ceb22180]      228 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.1 / 0.0 ms  (average mu = 0.843, current mu = 0.845) testing GC in old space requested
[227134:0x5560ceb22180]      242 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.2 / 0.0 ms  (average mu = 0.843, current mu = 0.843) testing GC in old space requested
[227134:0x5560ceb22180]      256 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.1 / 0.0 ms  (average mu = 0.845, current mu = 0.848) testing GC in old space requested
[227134:0x5560ceb22180]      270 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.1 / 0.0 ms  (average mu = 0.845, current mu = 0.846) testing GC in old space requested
[227134:0x5560ceb22180]      283 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.0 / 0.0 ms  (average mu = 0.849, current mu = 0.853) testing GC in old space requested
[227134:0x5560ceb22180]      297 ms: Mark-sweep 10.9 (25.2) -> 2.7 (22.7) MB, 2.0 / 0.0 ms  (average mu = 0.851, current mu = 0.853) testing GC in old space requested

@bounc3-paradise-on-e
Copy link
Author

bounc3-paradise-on-e commented Aug 8, 2018

@hashseed

Hm... What now? The crash is real in the sense that the code breaks Node.js process after some time. How can I help reproduce this? Should I file an issue at Node.js main repo?

In meanwhile I ran longer test using --inspect-brk. I created heapdumps before and after using chrome devtools inspector. After comparing the snapshots, nothing is really allocated on the v8 heap, but the overall memory (rss) grows in size constantly.

Do I need to provide Node.js process full memory dump (obtained using task manager), would it help? Since the issue (?) is introduced in Node.js v6.0.0, can I help somehow confirm it or bisect to specified commit in order to reveal what may cause it?

Thanks for cooperation so far.

@hashseed
Copy link
Member

At least with V8's own shell, I wasn't able to reproduce a memory growth with the newest V8 version. I can however reproduce very slow growth in Node.js itself. Seems like a bug in Node. I filed an issue against Node.js.

@bounc3-paradise-on-e
Copy link
Author

Thanks. I'm closing this.

@bounc3-paradise-on-e bounc3-paradise-on-e changed the title Access variables belonging to the parent scope using a C++ module Memory leak in Node.js Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants