module: fix the leak in SourceTextModule and ContextifySript

This is a POC that shows how we can replace the persistent
handles to v8::Module and v8::UnboundScript with an internal
reference and fix the leaks.

Author: joyeecheung

src/module_wrap.cc

@@ -53,18 +53,21 @@ ModuleWrap::ModuleWrap(Environment* env,
                        Local<Object> object,
                        Local<Module> module,
                        Local<String> url)
-    : BaseObject(env, object), module_(env->isolate(), module) {
+    : BaseObject(env, object),
+      module_(env->isolate(), module),
+      module_hash_(module->GetIdentityHash()) {
   Local<Value> undefined = Undefined(env->isolate());
+  object->SetDataInInternalField(kModuleSlot, module);
   object->SetInternalField(kURLSlot, url);
   object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined);
   object->SetInternalField(kContextObjectSlot, undefined);
   MakeWeak();
+  module_.SetWeak();
 }
 
 ModuleWrap::~ModuleWrap() {
   HandleScope scope(env()->isolate());
-  Local<Module> module = module_.Get(env()->isolate());
-  auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash());
+  auto range = env()->hash_to_module_map.equal_range(module_hash_);
   for (auto it = range.first; it != range.second; ++it) {
     if (it->second == this) {
       env()->hash_to_module_map.erase(it);

src/module_wrap.h

@@ -33,7 +33,7 @@ enum HostDefinedOptions : int {
 class ModuleWrap : public BaseObject {
  public:
   enum InternalFields {
-    kModuleWrapBaseField = BaseObject::kInternalFieldCount,
+    kModuleSlot = BaseObject::kInternalFieldCount,
     kURLSlot,
     kSyntheticEvaluationStepsSlot,
     kContextObjectSlot,  // Object whose creation context is the target Context
@@ -104,6 +104,7 @@ class ModuleWrap : public BaseObject {
   contextify::ContextifyContext* contextify_context_ = nullptr;
   bool synthetic_ = false;
   bool linked_ = false;
+  int module_hash_;
 };
 
 }  // namespace loader

src/node_contextify.cc

@@ -862,7 +862,11 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
                      "ContextifyScript::New");
     return;
   }
+
   contextify_script->script_.Reset(isolate, v8_script);
+  contextify_script->script_.SetWeak();
+  contextify_script->object()->SetDataInInternalField(kUnboundScriptSlot,
+                                                      v8_script);
 
   std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
   if (produce_cached_data) {

src/node_contextify.h

@@ -151,6 +151,11 @@ class ContextifyContext : public BaseObject {
 
 class ContextifyScript : public BaseObject {
  public:
+  enum InternalFields {
+    kUnboundScriptSlot = BaseObject::kInternalFieldCount,
+    kInternalFieldCount
+  };
+
   SET_NO_MEMORY_INFO()
   SET_MEMORY_INFO_NAME(ContextifyScript)
   SET_SELF_SIZE(ContextifyScript)

test/es-module/test-vm-contextified-script-leak.js

@@ -0,0 +1,17 @@
+'use strict';
+
+// Flags: --max-old-space-size=16 --trace-gc
+
+require('../common');
+const vm = require('vm');
+let count = 0;
+
+function main() {
+  new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
+    async importModuleDynamically() {},
+  });
+  if (count++ < 2 * 1024) {
+    setTimeout(main, 1);
+  }
+}
+main();

test/es-module/test-vm-source-text-module-leak.js

@@ -0,0 +1,24 @@
+'use strict';
+
+// This tests that vm.SourceTextModule() does not leak.
+// See: https://github.com/nodejs/node/issues/33439
+// Flags: --experimental-vm-modules --max-old-space-size=16 --trace-gc
+
+require('../common');
+
+const vm = require('vm');
+let count = 0;
+async function createModule() {
+  const m = new vm.SourceTextModule(`
+  const bar = new Array(512).fill("----");
+  export { bar };
+`);
+  await m.link(() => {});
+  await m.evaluate();
+  count++;
+  if (count < 4096) {
+    setTimeout(createModule, 1);
+  }
+  return m;
+}
+createModule();