From 59a075e108c7ffcee87252255e60530cd15a8adb Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 14 Sep 2013 18:29:24 +0400 Subject: [PATCH] contextify: fix ContextifyContext leak Apparently, context->Global() won't be destroyed if the context itself isn't marked as weak and independent. Also, the weakness flag should be cleared once the weak callback is executed, otherwise we'll get crashes in Debug builds. fix #6115 and #6201 --- src/node_contextify.cc | 18 +++++++++++------- test/pummel/test-vm-memleak.js | 7 +++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 1c9d9beb3de..c76b99d25c2 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -66,11 +66,13 @@ class ContextifyContext { , sandbox_(env->isolate(), sandbox) , context_(env->isolate(), CreateV8Context(env)) , proxy_global_(env->isolate(), context()->Global()) - // Wait for both sandbox_'s and proxy_global_'s death - , references_(2) { - sandbox_.MakeWeak(this, SandboxFreeCallback); + // Wait for sandbox_, proxy_global_, and context_ to die + , references_(3) { + sandbox_.MakeWeak(this, WeakCallback); sandbox_.MarkIndependent(); - proxy_global_.MakeWeak(this, SandboxFreeCallback); + context_.MakeWeak(this, WeakCallback); + context_.MarkIndependent(); + proxy_global_.MakeWeak(this, WeakCallback); proxy_global_.MarkIndependent(); } @@ -175,9 +177,11 @@ class ContextifyContext { } - static void SandboxFreeCallback(Isolate* isolate, - Persistent* target, - ContextifyContext* context) { + template + static void WeakCallback(Isolate* isolate, + Persistent* target, + ContextifyContext* context) { + target->ClearWeak(); if (--context->references_ == 0) delete context; } diff --git a/test/pummel/test-vm-memleak.js b/test/pummel/test-vm-memleak.js index e0b087d81b2..2abedc297b7 100644 --- a/test/pummel/test-vm-memleak.js +++ b/test/pummel/test-vm-memleak.js @@ -38,9 +38,16 @@ var interval = setInterval(function() { if (Date.now() - start > 5 * 1000) { // wait 10 seconds. clearInterval(interval); + + testContextLeak(); } }, 1); +function testContextLeak() { + for (var i = 0; i < 1000; i++) + require('vm').createContext({}); +} + process.on('exit', function() { console.error('max mem: %dmb', Math.round(maxMem / (1024 * 1024))); // make sure we stay below 100mb