From f7c8aa4cf17f427bee6b1989aba6f2714c9f0d34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Wed, 15 Mar 2023 15:35:06 +0100
Subject: [PATCH] Revert "vm: fix leak in vm.compileFunction when
 importModuleDynamically is used"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 986498b7b329f454507b3a47e9f20cbcdb029dc2.

Fixes: https://github.com/nodejs/node/issues/47096
PR-URL: https://github.com/nodejs/node/pull/47101
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 src/env_properties.h                         |  1 -
 src/node_contextify.cc                       | 17 ++++++++++++-----
 src/node_contextify.h                        |  3 +++
 test/pummel/test-vm-compile-function-leak.js | 14 --------------
 4 files changed, 15 insertions(+), 20 deletions(-)
 delete mode 100644 test/pummel/test-vm-compile-function-leak.js

diff --git a/src/env_properties.h b/src/env_properties.h
index 5bc4c838b44e74..65ab65de3ac3fd 100644
--- 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
index bd46a4a1e50e86..5aadefd84d6af7 100644
--- 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
index a9b7741e61b2c2..76c89318bb6cbf 100644
--- 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
deleted file mode 100644
index 465f300d4310d1..00000000000000
--- a/test/pummel/test-vm-compile-function-leak.js
+++ /dev/null
@@ -1,14 +0,0 @@
-'use strict';
-
-// Flags: --max-old-space-size=10
-
-require('../common');
-const vm = require('vm');
-
-const code = `console.log("${'hello world '.repeat(1e5)}");`;
-
-for (let i = 0; i < 10000; i++) {
-  vm.compileFunction(code, [], {
-    importModuleDynamically: () => {},
-  });
-}