5 ms·

Wait, is #3 for real? If that's the case it seems like a huge oversight.

IgorPartola 9y ago
Yeah, of the four things listed, I think it's the one that would trip me up the most. I think the safety net here would be dead code elimination: `unused()` should be detected as never called and removed during compilation/transpilation so that this wouldn't be an issue. I am sort of surprised that `unused` is not picked up by the garbage collector in the first place though. Since JS functions are objects, shouldn't it detect an object that's not referenced during the mark and sweep? In general, I really hate having to debug memory leaks in JS or Python. The interpreter for both will randomly allocate additional memory as it runs, so using tools like Valgrind is next to impossible. The only reliable method I've found is to pepper my code with logging statements that show what the current memory usage is, run the code like 1000-10,000 times, and see the points between which the memory usage goes up without coming down on a consistent basis. Python's built in `gc` module seems nearly useless for determining what's actually stuck in memory, and having a billion libraries that can have their own memory leaks is also not fun. These are the times I miss C: when you leak memory in C you know it because it becomes painful fast and it's usually easy to find, if your code is sane.
- favorited 9y ago
  I haven't done JS in a while, but it sounds to me like it is referenced, just not in source code. `someMethod` references it implicitly. Is my understanding correct?
  - gsnedders 9y ago
    Yes. The function's frame contains a reference to the object storing local variables of the parent frame. To do better you need to store a list of variables referenced (which you can only do if there's neither direct-eval nor a with statement).
- Ajedi32 9y ago
  > In general, I really hate having to debug memory leaks in JS or Python. The interpreter for both will randomly allocate additional memory as it runs, so using tools like Valgrind is next to impossible. I don't know about Python, but for JavaScript can't you use the built-in devtools? Maybe try grabbing a heap snapshot, or recording a record allocation profile and go from there?
- wahern 9y ago
  The fix is to implement the closures properly, by only closing over individual variable slots. It looks like the engines are implementing closures by closing over entire windows of slots--that is, if two functions have the same scope, they inherit the _union_ of the variables they reference as a single window/block of variable slots. The original article has a much simpler explanation and solution: https://blog.meteor.com/an-interesting-kind-of-javascript-memory-leak-8b47d2e7f156 https://blog.meteor.com/an-interesting-kind-of-javascript-me... To learn more about closures than you ever thought possible, try reading this paper describing how closures are implemented in Lua: http://www.cs.tufts.edu/~nr/cs257/archive/roberto-ierusalimschy/closures-draft.pdf http://www.cs.tufts.edu/~nr/cs257/archive/roberto-ierusalims...
paulddraper 9y ago
Yeah. I thought so too (and still think so). SO post: https://stackoverflow.com/questions/19798803/how-javascript-closures-are-garbage-collected https://stackoverflow.com/questions/19798803/how-javascript-... Chrome bug report: http://crbug.com/315190 http://crbug.com/315190 Meteor blog (linked in article): https://blog.meteor.com/an-interesting-kind-of-javascript-memory-leak-8b47d2e7f156 https://blog.meteor.com/an-interesting-kind-of-javascript-me... Live example (will crash due to memory leak): https://s3.amazonaws.com/chromebugs/memory.html https://s3.amazonaws.com/chromebugs/memory.html --- The reason this exists in all JS engines is for performance; it's easier to have on context record instead of several. Other languages do not do this. Off the top of my head: Lua, Java, Scala
- vvanders 9y ago
  Yeah, there's a nice link in the comments on the chrome bug on how Lua does it with upvalues: https://bugs.chromium.org/p/chromium/issues/detail?id=315190#c14 https://bugs.chromium.org/p/chromium/issues/detail?id=315190...
- dingo_bat 9y ago
  > Live example (will crash due to memory leak): https://s3.amazonaws.com/chromebugs/memory.html https://s3.amazonaws.com/chromebugs/memory.html Happy to report that no crash in Nightly. Edit: No crash in Edge either.
  - paulddraper 9y ago
    It'll crash Chrome because it puts stricter limits on JS memory. (Or something.) Firefox and Edge won't crash, but you'll be using 3GB+.
    - dingo_bat 9y ago
      Yup Firefox was at 3.6GB and edge at 2.8GB.
maxxxxx 9y ago
Would it be better to have to explicitly declare what variables you want to import into the closure like PHP or C++ do? C# also captures everything by default and reference which has tripped me up quite a few times.
- jhgb 9y ago
  That might help but it seems like either an implementation or language spec fix is in order. There doesn't seem to be a reason for a function without free variables to turn into a closure at all, thus preventing the issue.
  - paulddraper 9y ago
    Yep. Currently, the ECMAScript specification says nothing about GC. And it seems every major JS engine has decided that this type of memory leak is okay. So it's rather unlikely something will change.
Jach 9y ago
The particular memory issue is new to me (though now I can watch out for it, yay) but I'm not surprised... JS lacking proper lexical scope causes many issues.
- chasd00 9y ago
  when you say "proper lexical scope" do you mean just block vs function level scoping of variables? If so, i wouldn't say javascript is wrong it's just different.
- barrkel 9y ago
  Lexical scope is a semantics concern - observable behaviour that is required for correctness. The case under discussion is an implementation concern - there's no necessity for it to leak by creating a linked list of activation records as further analysis could break the chain. The two are not related.
bzbarsky 9y ago
It's for real, in V8. Other JS implementations may not have the same problem.

Yeah, of the four things listed, I think it's the one that would trip me up the most. I think the safety net here would be dead code elimination: `unused()` should be detected as never called and removed during compilation/transpilation so that this wouldn't be an issue. I am sort of surprised that `unused` is not picked up by the garbage collector in the first place though. Since JS functions are objects, shouldn't it detect an object that's not referenced during the mark and sweep? In general, I really hate having to debug memory leaks in JS or Python. The interpreter for both will randomly allocate additional memory as it runs, so using tools like Valgrind is next to impossible. The only reliable method I've found is to pepper my code with logging statements that show what the current memory usage is, run the code like 1000-10,000 times, and see the points between which the memory usage goes up without coming down on a consistent basis. Python's built in `gc` module seems nearly useless for determining what's actually stuck in memory, and having a billion libraries that can have their own memory leaks is also not fun. These are the times I miss C: when you leak memory in C you know it because it becomes painful fast and it's usually easy to find, if your code is sane.

favorited 9y ago
I haven't done JS in a while, but it sounds to me like it is referenced, just not in source code. `someMethod` references it implicitly. Is my understanding correct?
- gsnedders 9y ago
  Yes. The function's frame contains a reference to the object storing local variables of the parent frame. To do better you need to store a list of variables referenced (which you can only do if there's neither direct-eval nor a with statement).
Ajedi32 9y ago
> In general, I really hate having to debug memory leaks in JS or Python. The interpreter for both will randomly allocate additional memory as it runs, so using tools like Valgrind is next to impossible. I don't know about Python, but for JavaScript can't you use the built-in devtools? Maybe try grabbing a heap snapshot, or recording a record allocation profile and go from there?
wahern 9y ago
The fix is to implement the closures properly, by only closing over individual variable slots. It looks like the engines are implementing closures by closing over entire windows of slots--that is, if two functions have the same scope, they inherit the _union_ of the variables they reference as a single window/block of variable slots. The original article has a much simpler explanation and solution: https://blog.meteor.com/an-interesting-kind-of-javascript-memory-leak-8b47d2e7f156 https://blog.meteor.com/an-interesting-kind-of-javascript-me... To learn more about closures than you ever thought possible, try reading this paper describing how closures are implemented in Lua: http://www.cs.tufts.edu/~nr/cs257/archive/roberto-ierusalimschy/closures-draft.pdf http://www.cs.tufts.edu/~nr/cs257/archive/roberto-ierusalims...

paulddraper 9y ago

Yeah. I thought so too (and still think so). SO post: https://stackoverflow.com/questions/19798803/how-javascript-closures-are-garbage-collected https://stackoverflow.com/questions/19798803/how-javascript-... Chrome bug report: http://crbug.com/315190 http://crbug.com/315190 Meteor blog (linked in article): https://blog.meteor.com/an-interesting-kind-of-javascript-memory-leak-8b47d2e7f156 https://blog.meteor.com/an-interesting-kind-of-javascript-me... Live example (will crash due to memory leak): https://s3.amazonaws.com/chromebugs/memory.html https://s3.amazonaws.com/chromebugs/memory.html --- The reason this exists in all JS engines is for performance; it's easier to have on context record instead of several. Other languages do not do this. Off the top of my head: Lua, Java, Scala

vvanders 9y ago
Yeah, there's a nice link in the comments on the chrome bug on how Lua does it with upvalues: https://bugs.chromium.org/p/chromium/issues/detail?id=315190#c14 https://bugs.chromium.org/p/chromium/issues/detail?id=315190...
dingo_bat 9y ago
> Live example (will crash due to memory leak): https://s3.amazonaws.com/chromebugs/memory.html https://s3.amazonaws.com/chromebugs/memory.html Happy to report that no crash in Nightly. Edit: No crash in Edge either.
- paulddraper 9y ago
  It'll crash Chrome because it puts stricter limits on JS memory. (Or something.) Firefox and Edge won't crash, but you'll be using 3GB+.
  - dingo_bat 9y ago
    Yup Firefox was at 3.6GB and edge at 2.8GB.

maxxxxx 9y ago

Would it be better to have to explicitly declare what variables you want to import into the closure like PHP or C++ do? C# also captures everything by default and reference which has tripped me up quite a few times.

jhgb 9y ago
That might help but it seems like either an implementation or language spec fix is in order. There doesn't seem to be a reason for a function without free variables to turn into a closure at all, thus preventing the issue.
- paulddraper 9y ago
  Yep. Currently, the ECMAScript specification says nothing about GC. And it seems every major JS engine has decided that this type of memory leak is okay. So it's rather unlikely something will change.

Jach 9y ago

The particular memory issue is new to me (though now I can watch out for it, yay) but I'm not surprised... JS lacking proper lexical scope causes many issues.

chasd00 9y ago
when you say "proper lexical scope" do you mean just block vs function level scoping of variables? If so, i wouldn't say javascript is wrong it's just different.
barrkel 9y ago
Lexical scope is a semantics concern - observable behaviour that is required for correctness. The case under discussion is an implementation concern - there's no necessity for it to leak by creating a linked list of activation records as further analysis could break the chain. The two are not related.

bzbarsky 9y ago

It's for real, in V8. Other JS implementations may not have the same problem.