Browse files

Fix memleak in vm.runInNewContext

Closes GH-704.
  • Loading branch information...
1 parent df15472 commit 1f50d711b25c15228b4c85da0e44075239e17e68 @ry ry committed Feb 24, 2011
Showing with 35 additions and 1 deletion.
  1. +8 −1 src/node_script.cc
  2. +27 −0 test/pummel/test-vm-memleak.js
View
9 src/node_script.cc
@@ -355,7 +355,14 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
if (output_flag == returnResult) {
result = script->Run();
- if (result.IsEmpty()) return try_catch.ReThrow();
+ if (result.IsEmpty()) {
+ if (context_flag == newContext) {
+ context->DetachGlobal();
+ context->Exit();
+ context.Dispose();
+ }
+ return try_catch.ReThrow();
+ }
} else {
WrappedScript *n_script = ObjectWrap::Unwrap<WrappedScript>(args.Holder());
if (!n_script) {
View
27 test/pummel/test-vm-memleak.js
@@ -0,0 +1,27 @@
+var assert = require('assert');
+var common = require('../common');
+
+var start = Date.now();
+var maxMem = 0;
+
+var interval = setInterval(function() {
+ try {
+ require('vm').runInNewContext('throw 1;');
+ } catch(e) {
+ }
+
+ var rss = process.memoryUsage().rss;
+ maxMem = Math.max(rss, maxMem);
+
+
+ if (Date.now() - start > 5*1000) {
+ // wait 10 seconds.
+ clearInterval(interval);
+ }
+}, 1);
+
+process.on('exit', function() {
+ console.error('max mem: %dmb', Math.round(maxMem / (1024*1024)));
+ // make sure we stay below 100mb
+ assert.ok(maxMem < 50 * 1024 * 1024);
+});

0 comments on commit 1f50d71

Please sign in to comment.