Revert "vm: fix leak in vm.compileFunction when importModuleDynamical…

…ly is used" This reverts commit 986498b. Fixes: #47096 PR-URL: #47101 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Danielle Adams <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Jan 30, 2024 · c0f1ff2 · c0f1ff2
1 parent 277f7c6
commit c0f1ff2
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 20 deletions.
diff --git a/src/env_properties.h b/src/env_properties.h
@@ -20,7 +20,6 @@
 #define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)                               \
   V(arrow_message_private_symbol, "node:arrowMessage")                         \
   V(contextify_context_private_symbol, "node:contextify:context")              \
-  V(compiled_function_entry, "node:compiled_function_entry")                   \
   V(decorated_private_symbol, "node:decorated")                                \
   V(napi_type_tag, "node:napi:type_tag")                                       \
   V(napi_wrapper, "node:napi:wrapper")                                         \

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -77,6 +77,7 @@ using v8::Uint32;
 using v8::UnboundScript;
 using v8::Value;
 using v8::WeakCallbackInfo;
+using v8::WeakCallbackType;
 
 // The vm module executes code in a sandboxed environment with a different
 // global object than the rest of the code. This is achieved by applying
@@ -1262,7 +1263,8 @@ void ContextifyContext::CompileFunction(
            context).ToLocal(&cache_key)) {
     return;
   }
-  new CompiledFnEntry(env, cache_key, id, fn);
+  CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, fn);
+  env->id_to_function_map.emplace(id, entry);
 
   Local<Object> result = Object::New(isolate);
   if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
@@ -1294,18 +1296,23 @@ void ContextifyContext::CompileFunction(
   args.GetReturnValue().Set(result);
 }
 
+void CompiledFnEntry::WeakCallback(
+    const WeakCallbackInfo<CompiledFnEntry>& data) {
+  CompiledFnEntry* entry = data.GetParameter();
+  delete entry;
+}
+
 CompiledFnEntry::CompiledFnEntry(Environment* env,
                                  Local<Object> object,
                                  uint32_t id,
                                  Local<Function> fn)
-    : BaseObject(env, object), id_(id) {
-  MakeWeak();
-  fn->SetPrivate(env->context(), env->compiled_function_entry(), object);
-  env->id_to_function_map.emplace(id, this);
+    : BaseObject(env, object), id_(id), fn_(env->isolate(), fn) {
+  fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
 }
 
 CompiledFnEntry::~CompiledFnEntry() {
   env()->id_to_function_map.erase(id_);
+  fn_.ClearWeak();
 }
 
 static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {

diff --git a/src/node_contextify.h b/src/node_contextify.h
@@ -194,6 +194,9 @@ class CompiledFnEntry final : public BaseObject {
 
  private:
   uint32_t id_;
+  v8::Global<v8::Function> fn_;
+
+  static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
 };
 
 v8::Maybe<bool> StoreCodeCacheResult(

diff --git a/test/pummel/test-vm-compile-function-leak.js b/test/pummel/test-vm-compile-function-leak.js