From 85cf80507f41b11167b95a8bb19083159543f6e5 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 7 Feb 2022 16:28:39 +0100
Subject: [PATCH] LibJS: Make ScriptOrModule use WeakPtr instead of raw
 pointers

---
 Userland/Libraries/LibJS/AST.cpp                    |  7 ++++---
 Userland/Libraries/LibJS/CyclicModule.cpp           |  4 ++--
 Userland/Libraries/LibJS/Interpreter.cpp            |  2 +-
 Userland/Libraries/LibJS/Module.h                   |  4 +++-
 Userland/Libraries/LibJS/Runtime/ExecutionContext.h |  2 +-
 Userland/Libraries/LibJS/Runtime/VM.cpp             |  4 ++--
 Userland/Libraries/LibJS/Script.h                   |  4 +++-
 Userland/Libraries/LibJS/SourceTextModule.cpp       | 12 ++++++------
 Userland/Libraries/LibJS/SyntheticModule.cpp        |  2 +-
 9 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index 62ae748dc9..3bde7974b9 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -3175,9 +3175,10 @@ Completion MetaProperty::execute(Interpreter& interpreter, GlobalObject& global_
         auto script_or_module = interpreter.vm().get_active_script_or_module();
 
         // 2. Assert: module is a Source Text Module Record.
-        VERIFY(script_or_module.has<Module*>());
-        VERIFY(is<SourceTextModule>(*script_or_module.get<Module*>()));
-        auto& module = static_cast<SourceTextModule&>(*script_or_module.get<Module*>());
+        VERIFY(script_or_module.has<WeakPtr<Module>>());
+        VERIFY(script_or_module.get<WeakPtr<Module>>());
+        VERIFY(is<SourceTextModule>(*script_or_module.get<WeakPtr<Module>>()));
+        auto& module = static_cast<SourceTextModule&>(*script_or_module.get<WeakPtr<Module>>());
 
         // 3. Let importMeta be module.[[ImportMeta]].
         auto* import_meta = module.import_meta();
diff --git a/Userland/Libraries/LibJS/CyclicModule.cpp b/Userland/Libraries/LibJS/CyclicModule.cpp
index 9276d01be2..3414103e80 100644
--- a/Userland/Libraries/LibJS/CyclicModule.cpp
+++ b/Userland/Libraries/LibJS/CyclicModule.cpp
@@ -107,7 +107,7 @@ ThrowCompletionOr<u32> CyclicModule::inner_module_linking(VM& vm, Vector<Module*
         ModuleRequest required { required_string };
 
         // a. Let requiredModule be ? HostResolveImportedModule(module, required).
-        auto required_module = TRY(vm.host_resolve_imported_module(this, required));
+        auto required_module = TRY(vm.host_resolve_imported_module(this->make_weak_ptr(), required));
 
         // b. Set index to ? InnerModuleLinking(requiredModule, stack, index).
         index = TRY(required_module->inner_module_linking(vm, stack, index));
@@ -310,7 +310,7 @@ ThrowCompletionOr<u32> CyclicModule::inner_module_evaluation(VM& vm, Vector<Modu
     for (auto& required : m_requested_modules) {
 
         // a. Let requiredModule be ! HostResolveImportedModule(module, required).
-        auto* required_module = MUST(vm.host_resolve_imported_module(this, required)).ptr();
+        auto* required_module = MUST(vm.host_resolve_imported_module(this->make_weak_ptr(), required)).ptr();
         // b. NOTE: Link must be completed successfully prior to invoking this method, so every requested module is guaranteed to resolve successfully.
 
         // c. Set index to ? InnerModuleEvaluation(requiredModule, stack, index).
diff --git a/Userland/Libraries/LibJS/Interpreter.cpp b/Userland/Libraries/LibJS/Interpreter.cpp
index 8fda3bad4c..1e7bf35847 100644
--- a/Userland/Libraries/LibJS/Interpreter.cpp
+++ b/Userland/Libraries/LibJS/Interpreter.cpp
@@ -63,7 +63,7 @@ ThrowCompletionOr<Value> Interpreter::run(Script& script_record)
     script_context.realm = &script_record.realm();
 
     // 5. Set the ScriptOrModule of scriptContext to scriptRecord.
-    script_context.script_or_module = &script_record;
+    script_context.script_or_module = script_record.make_weak_ptr();
 
     // 6. Set the VariableEnvironment of scriptContext to globalEnv.
     script_context.variable_environment = &global_environment;
diff --git a/Userland/Libraries/LibJS/Module.h b/Userland/Libraries/LibJS/Module.h
index 820f6af0e4..051f0cb7f7 100644
--- a/Userland/Libraries/LibJS/Module.h
+++ b/Userland/Libraries/LibJS/Module.h
@@ -53,7 +53,9 @@ struct ResolvedBinding {
 };
 
 // 16.2.1.4 Abstract Module Records, https://tc39.es/ecma262/#sec-abstract-module-records
-class Module : public RefCounted<Module> {
+class Module
+    : public RefCounted<Module>
+    , public Weakable<Module> {
 public:
     virtual ~Module();
 
diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h
index 8f247666ca..a102982aaa 100644
--- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h
+++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h
@@ -15,7 +15,7 @@
 
 namespace JS {
 
-using ScriptOrModule = Variant<Empty, Script*, Module*>;
+using ScriptOrModule = Variant<Empty, WeakPtr<Script>, WeakPtr<Module>>;
 
 // 9.4 Execution Contexts, https://tc39.es/ecma262/#sec-execution-contexts
 struct ExecutionContext {
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index f4e116c96e..1444a31974 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -770,7 +770,7 @@ ThrowCompletionOr<void> VM::link_and_eval_module(Module& module)
             dbgln("Warning: Using multiple modules as entry point can lead to unexpected results");
 
         m_loaded_modules.empend(
-            &module,
+            module.make_weak_ptr(),
             module.filename(),
             String {}, // Null type
             module,
@@ -836,7 +836,7 @@ ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrMod
         [&](Empty) {
             return "."sv;
         },
-        [&](auto* script_or_module) {
+        [&](auto& script_or_module) {
             return script_or_module->filename();
         });
 
diff --git a/Userland/Libraries/LibJS/Script.h b/Userland/Libraries/LibJS/Script.h
index 782627e99f..6fa37e8dd3 100644
--- a/Userland/Libraries/LibJS/Script.h
+++ b/Userland/Libraries/LibJS/Script.h
@@ -16,7 +16,9 @@
 namespace JS {
 
 // 16.1.4 Script Records, https://tc39.es/ecma262/#sec-script-records
-class Script : public RefCounted<Script> {
+class Script
+    : public RefCounted<Script>
+    , public Weakable<Script> {
 public:
     ~Script();
     static Result<NonnullRefPtr<Script>, Vector<Parser::Error>> parse(StringView source_text, Realm&, StringView filename = {});
diff --git a/Userland/Libraries/LibJS/SourceTextModule.cpp b/Userland/Libraries/LibJS/SourceTextModule.cpp
index d9cf6e0fbb..066e97ac89 100644
--- a/Userland/Libraries/LibJS/SourceTextModule.cpp
+++ b/Userland/Libraries/LibJS/SourceTextModule.cpp
@@ -280,7 +280,7 @@ ThrowCompletionOr<Vector<FlyString>> SourceTextModule::get_exported_names(VM& vm
     // 7. For each ExportEntry Record e of module.[[StarExportEntries]], do
     for (auto& entry : m_star_export_entries) {
         // a. Let requestedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]).
-        auto requested_module = TRY(vm.host_resolve_imported_module(this, entry.module_request()));
+        auto requested_module = TRY(vm.host_resolve_imported_module(this->make_weak_ptr(), entry.module_request()));
 
         // b. Let starNames be ? requestedModule.GetExportedNames(exportStarSet).
         auto star_names = TRY(requested_module->get_exported_names(vm, export_star_set));
@@ -336,7 +336,7 @@ Completion SourceTextModule::initialize_environment(VM& vm)
     // 7. For each ImportEntry Record in of module.[[ImportEntries]], do
     for (auto& import_entry : m_import_entries) {
         // a. Let importedModule be ! HostResolveImportedModule(module, in.[[ModuleRequest]]).
-        auto imported_module = MUST(vm.host_resolve_imported_module(this, import_entry.module_request()));
+        auto imported_module = MUST(vm.host_resolve_imported_module(this->make_weak_ptr(), import_entry.module_request()));
         // b. NOTE: The above call cannot fail because imported module requests are a subset of module.[[RequestedModules]], and these have been resolved earlier in this algorithm.
 
         // c. If in.[[ImportName]] is namespace-object, then
@@ -390,7 +390,7 @@ Completion SourceTextModule::initialize_environment(VM& vm)
     m_execution_context.realm = &realm();
 
     // 12. Set the ScriptOrModule of moduleContext to module.
-    m_execution_context.script_or_module = this;
+    m_execution_context.script_or_module = this->make_weak_ptr();
 
     // 13. Set the VariableEnvironment of moduleContext to module.[[Environment]].
     m_execution_context.variable_environment = environment;
@@ -536,7 +536,7 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
             continue;
 
         // i. Let importedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]).
-        auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request()));
+        auto imported_module = TRY(vm.host_resolve_imported_module(this->make_weak_ptr(), entry.module_request()));
 
         // ii. If e.[[ImportName]] is all, then
         if (entry.kind == ExportStatement::ExportEntry::Kind::ModuleRequestAll) {
@@ -576,7 +576,7 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
     // 8. For each ExportEntry Record e of module.[[StarExportEntries]], do
     for (auto& entry : m_star_export_entries) {
         // a. Let importedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]).
-        auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request()));
+        auto imported_module = TRY(vm.host_resolve_imported_module(this->make_weak_ptr(), entry.module_request()));
 
         // b. Let resolution be ? importedModule.ResolveExport(exportName, resolveSet).
         auto resolution = TRY(imported_module->resolve_export(vm, export_name, resolve_set));
@@ -638,7 +638,7 @@ Completion SourceTextModule::execute_module(VM& vm, Optional<PromiseCapability>
     module_context.realm = &realm();
 
     // 4. Set the ScriptOrModule of moduleContext to module.
-    module_context.script_or_module = this;
+    module_context.script_or_module = this->make_weak_ptr();
 
     // 5. Assert: module has been linked and declarations in its module environment have been instantiated.
     VERIFY(m_status != ModuleStatus::Unlinked && m_status != ModuleStatus::Linking && environment());
diff --git a/Userland/Libraries/LibJS/SyntheticModule.cpp b/Userland/Libraries/LibJS/SyntheticModule.cpp
index 99f693b8c2..0d3e6249bb 100644
--- a/Userland/Libraries/LibJS/SyntheticModule.cpp
+++ b/Userland/Libraries/LibJS/SyntheticModule.cpp
@@ -89,7 +89,7 @@ ThrowCompletionOr<Promise*> JS::SyntheticModule::evaluate(VM& vm)
     module_context.realm = &realm();
 
     // 5. Set the ScriptOrModule of moduleContext to module.
-    module_context.script_or_module = this;
+    module_context.script_or_module = this->make_weak_ptr();
 
     // 6. Set the VariableEnvironment of moduleContext to module.[[Environment]].
     module_context.variable_environment = environment();