src: add cleanup hook for ContextifyContext

Otherwise there’s a memory leak left by the context when the Isolate tears down without having run the weak callback. PR-URL: #28631 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
nodejs · Jul 20, 2019 · 5c1d595 · 5c1d595
1 parent 03de306
commit 5c1d595
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 0 deletions.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -114,6 +114,19 @@ ContextifyContext::ContextifyContext(
 
   context_.Reset(env->isolate(), v8_context.ToLocalChecked());
   context_.SetWeak(this, WeakCallback, WeakCallbackType::kParameter);
+  env->AddCleanupHook(CleanupHook, this);
+}
+
+
+ContextifyContext::~ContextifyContext() {
+  env()->RemoveCleanupHook(CleanupHook, this);
+}
+
+
+void ContextifyContext::CleanupHook(void* arg) {
+  ContextifyContext* self = static_cast<ContextifyContext*>(arg);
+  self->context_.Reset();
+  delete self;
 }
 
 

diff --git a/src/node_contextify.h b/src/node_contextify.h
@@ -22,6 +22,8 @@ class ContextifyContext {
   ContextifyContext(Environment* env,
                     v8::Local<v8::Object> sandbox_obj,
                     const ContextOptions& options);
+  ~ContextifyContext();
+  static void CleanupHook(void* arg);
 
   v8::MaybeLocal<v8::Object> CreateDataWrapper(Environment* env);
   v8::MaybeLocal<v8::Context> CreateV8Context(Environment* env,