Browse Source

LibJS: Make Script and Module GC-allocated

This ensures that code currently in any active or saved execution stack
always stays alive.
Andreas Kling 2 years ago
parent
commit
00c8f07192

+ 4 - 4
Userland/Libraries/LibJS/AST.cpp

@@ -3262,10 +3262,10 @@ Completion MetaProperty::execute(Interpreter& interpreter) const
         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<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>>());
+        VERIFY(script_or_module.has<NonnullGCPtr<Module>>());
+        VERIFY(script_or_module.get<NonnullGCPtr<Module>>());
+        VERIFY(is<SourceTextModule>(*script_or_module.get<NonnullGCPtr<Module>>()));
+        auto& module = static_cast<SourceTextModule&>(*script_or_module.get<NonnullGCPtr<Module>>());
 
         // 3. Let importMeta be module.[[ImportMeta]].
         auto* import_meta = module.import_meta();

+ 10 - 2
Userland/Libraries/LibJS/CyclicModule.cpp

@@ -17,6 +17,14 @@ CyclicModule::CyclicModule(Realm& realm, StringView filename, bool has_top_level
 {
 }
 
+void CyclicModule::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_cycle_root);
+    for (auto* module : m_async_parent_modules)
+        visitor.visit(module);
+}
+
 // 16.2.1.5.1 Link ( ), https://tc39.es/ecma262/#sec-moduledeclarationlinking
 ThrowCompletionOr<void> CyclicModule::link(VM& vm)
 {
@@ -106,7 +114,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->make_weak_ptr(), required));
+        auto required_module = TRY(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), required));
 
         // b. Set index to ? InnerModuleLinking(requiredModule, stack, index).
         index = TRY(required_module->inner_module_linking(vm, stack, index));
@@ -305,7 +313,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->make_weak_ptr(), required)).ptr();
+        auto* required_module = MUST(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), 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).

+ 5 - 1
Userland/Libraries/LibJS/CyclicModule.h

@@ -25,6 +25,8 @@ enum class ModuleStatus {
 
 // 16.2.1.5 Cyclic Module Records, https://tc39.es/ecma262/#sec-cyclic-module-records
 class CyclicModule : public Module {
+    JS_CELL(CyclicModule, Module);
+
 public:
     // Note: Do not call these methods directly unless you are HostResolveImportedModule.
     //       Badges cannot be used because other hosts must be able to call this (and it is called recursively)
@@ -34,6 +36,8 @@ public:
 protected:
     CyclicModule(Realm& realm, StringView filename, bool has_top_level_await, Vector<ModuleRequest> requested_modules);
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
     virtual ThrowCompletionOr<u32> inner_module_linking(VM& vm, Vector<Module*>& stack, u32 index) override;
     virtual ThrowCompletionOr<u32> inner_module_evaluation(VM& vm, Vector<Module*>& stack, u32 index) override;
 
@@ -50,7 +54,7 @@ protected:
     Optional<u32> m_dfs_index;                          // [[DFSIndex]]
     Optional<u32> m_dfs_ancestor_index;                 // [[DFSAncestorIndex]]
     Vector<ModuleRequest> m_requested_modules;          // [[RequestedModules]]
-    CyclicModule* m_cycle_root;                         // [[CycleRoot]]
+    CyclicModule* m_cycle_root { nullptr };             // [[CycleRoot]]
     bool m_has_top_level_await { false };               // [[HasTLA]]
     bool m_async_evaluation { false };                  // [[AsyncEvaluation]]
     Optional<PromiseCapability> m_top_level_capability; // [[TopLevelCapability]]

+ 1 - 1
Userland/Libraries/LibJS/Interpreter.cpp

@@ -54,7 +54,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.make_weak_ptr();
+    script_context.script_or_module = NonnullGCPtr<Script>(script_record);
 
     // 6. Set the VariableEnvironment of scriptContext to globalEnv.
     script_context.variable_environment = &global_environment;

+ 14 - 4
Userland/Libraries/LibJS/Module.cpp

@@ -12,11 +12,21 @@
 namespace JS {
 
 Module::Module(Realm& realm, String filename)
-    : m_realm(make_handle(&realm))
+    : m_realm(realm)
     , m_filename(move(filename))
 {
 }
 
+Module::~Module() = default;
+
+void Module::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_realm);
+    visitor.visit(m_environment);
+    visitor.visit(m_namespace);
+}
+
 // 16.2.1.5.1.1 InnerModuleLinking ( module, stack, index ), https://tc39.es/ecma262/#sec-InnerModuleLinking
 ThrowCompletionOr<u32> Module::inner_module_linking(VM& vm, Vector<Module*>&, u32 index)
 {
@@ -54,7 +64,7 @@ ThrowCompletionOr<Object*> Module::get_module_namespace(VM& vm)
     // FIXME: How do we check this without breaking encapsulation?
 
     // 2. Let namespace be module.[[Namespace]].
-    auto* namespace_ = m_namespace.is_null() ? nullptr : m_namespace.cell();
+    auto* namespace_ = m_namespace.ptr();
 
     // 3. If namespace is empty, then
     if (!namespace_) {
@@ -76,7 +86,7 @@ ThrowCompletionOr<Object*> Module::get_module_namespace(VM& vm)
 
         // d. Set namespace to ModuleNamespaceCreate(module, unambiguousNames).
         namespace_ = module_namespace_create(vm, unambiguous_names);
-        VERIFY(!m_namespace.is_null());
+        VERIFY(m_namespace);
         // Note: This set the local variable 'namespace' and not the member variable which is done by ModuleNamespaceCreate
     }
 
@@ -90,7 +100,7 @@ Object* Module::module_namespace_create(VM& vm, Vector<FlyString> unambiguous_na
     auto& realm = this->realm();
 
     // 1. Assert: module.[[Namespace]] is empty.
-    VERIFY(m_namespace.is_null());
+    VERIFY(!m_namespace);
 
     // 2. Let internalSlotsList be the internal slots listed in Table 34.
     // 3. Let M be MakeBasicObject(internalSlotsList).

+ 14 - 12
Userland/Libraries/LibJS/Module.h

@@ -8,7 +8,7 @@
 #pragma once
 
 #include <AK/FlyString.h>
-#include <LibJS/Heap/Handle.h>
+#include <LibJS/Heap/GCPtr.h>
 #include <LibJS/Runtime/Environment.h>
 #include <LibJS/Runtime/Realm.h>
 
@@ -55,18 +55,18 @@ struct ResolvedBinding {
 };
 
 // 16.2.1.4 Abstract Module Records, https://tc39.es/ecma262/#sec-abstract-module-records
-class Module
-    : public RefCounted<Module>
-    , public Weakable<Module> {
+class Module : public Cell {
+    JS_CELL(Module, Cell);
+
 public:
-    virtual ~Module() = default;
+    virtual ~Module() override;
 
-    Realm& realm() { return *m_realm.cell(); }
-    Realm const& realm() const { return *m_realm.cell(); }
+    Realm& realm() { return *m_realm; }
+    Realm const& realm() const { return *m_realm; }
 
     StringView filename() const { return m_filename; }
 
-    Environment* environment() { return m_environment.cell(); }
+    Environment* environment() { return m_environment; }
 
     ThrowCompletionOr<Object*> get_module_namespace(VM& vm);
 
@@ -82,9 +82,11 @@ public:
 protected:
     Module(Realm&, String filename);
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
     void set_environment(Environment* environment)
     {
-        m_environment = make_handle(environment);
+        m_environment = environment;
     }
 
 private:
@@ -95,9 +97,9 @@ private:
     // destroy the VM but keep the modules this should not happen. Because VM
     // stores modules with a RefPtr we cannot just store the VM as that leads to
     // cycles.
-    Handle<Realm> m_realm;             // [[Realm]]
-    Handle<Environment> m_environment; // [[Environment]]
-    Handle<Object> m_namespace;        // [[Namespace]]
+    GCPtr<Realm> m_realm;             // [[Realm]]
+    GCPtr<Environment> m_environment; // [[Environment]]
+    GCPtr<Object> m_namespace;        // [[Namespace]]
 
     // Needed for potential lookups of modules.
     String m_filename;

+ 6 - 0
Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -303,6 +303,12 @@ void ECMAScriptFunctionObject::visit_edges(Visitor& visitor)
         if (auto* property_key_ptr = field.name.get_pointer<PropertyKey>(); property_key_ptr && property_key_ptr->is_symbol())
             visitor.visit(property_key_ptr->as_symbol());
     }
+
+    m_script_or_module.visit(
+        [](Empty) {},
+        [&](auto& script_or_module) {
+            visitor.visit(script_or_module.ptr());
+        });
 }
 
 // 10.2.7 MakeMethod ( F, homeObject ), https://tc39.es/ecma262/#sec-makemethod

+ 1 - 1
Userland/Libraries/LibJS/Runtime/ExecutionContext.h

@@ -17,7 +17,7 @@
 
 namespace JS {
 
-using ScriptOrModule = Variant<Empty, WeakPtr<Script>, WeakPtr<Module>>;
+using ScriptOrModule = Variant<Empty, NonnullGCPtr<Script>, NonnullGCPtr<Module>>;
 
 // 9.4 Execution Contexts, https://tc39.es/ecma262/#sec-execution-contexts
 struct ExecutionContext {

+ 12 - 7
Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -205,6 +205,11 @@ void VM::gather_roots(HashTable<Cell*>& roots)
             roots.set(execution_context->lexical_environment);
             roots.set(execution_context->variable_environment);
             roots.set(execution_context->private_environment);
+            execution_context->script_or_module.visit(
+                [](Empty) {},
+                [&](auto& script_or_module) {
+                    roots.set(script_or_module.ptr());
+                });
         }
     };
 
@@ -800,7 +805,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.make_weak_ptr(),
+            NonnullGCPtr(module),
             module.filename(),
             String {}, // Null type
             module,
@@ -865,7 +870,7 @@ static String resolve_module_filename(StringView filename, StringView module_typ
 }
 
 // 16.2.1.7 HostResolveImportedModule ( referencingScriptOrModule, specifier ), https://tc39.es/ecma262/#sec-hostresolveimportedmodule
-ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrModule referencing_script_or_module, ModuleRequest const& module_request)
+ThrowCompletionOr<NonnullGCPtr<Module>> VM::resolve_imported_module(ScriptOrModule referencing_script_or_module, ModuleRequest const& module_request)
 {
     // An implementation of HostResolveImportedModule must conform to the following requirements:
     //  - If it completes normally, the [[Value]] slot of the completion must contain an instance of a concrete subclass of Module Record.
@@ -912,9 +917,9 @@ ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrMod
         },
         [&](auto& script_or_module) {
             if constexpr (IsSame<Script*, decltype(script_or_module)>) {
-                return String::formatted("Script @ {}", script_or_module);
+                return String::formatted("Script @ {}", script_or_module.ptr());
             }
-            return String::formatted("Module @ {}", script_or_module);
+            return String::formatted("Module @ {}", script_or_module.ptr());
         });
 
     dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] resolve_imported_module({}, {})", referencing_module_string, filename);
@@ -924,7 +929,7 @@ ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrMod
     auto* loaded_module_or_end = get_stored_module(referencing_script_or_module, filename, module_type);
     if (loaded_module_or_end != nullptr) {
         dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] resolve_imported_module({}) already loaded at {}", filename, loaded_module_or_end->module.ptr());
-        return loaded_module_or_end->module;
+        return NonnullGCPtr(*loaded_module_or_end->module);
     }
 
     dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] reading and parsing module {}", filename);
@@ -939,7 +944,7 @@ ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrMod
     auto file_content = file_or_error.value()->read_all();
     StringView content_view { file_content.data(), file_content.size() };
 
-    auto module = TRY([&]() -> ThrowCompletionOr<NonnullRefPtr<Module>> {
+    auto module = TRY([&]() -> ThrowCompletionOr<NonnullGCPtr<Module>> {
         // If assertions has an entry entry such that entry.[[Key]] is "type", let type be entry.[[Value]]. The following requirements apply:
         // If type is "json", then this algorithm must either invoke ParseJSONModule and return the resulting Completion Record, or throw an exception.
         if (module_type == "json"sv) {
@@ -965,7 +970,7 @@ ThrowCompletionOr<NonnullRefPtr<Module>> VM::resolve_imported_module(ScriptOrMod
         referencing_script_or_module,
         filename,
         module_type,
-        module,
+        *module,
         false);
 
     return module;

+ 3 - 3
Userland/Libraries/LibJS/Runtime/VM.h

@@ -220,7 +220,7 @@ public:
 
     ScriptOrModule get_active_script_or_module() const;
 
-    Function<ThrowCompletionOr<NonnullRefPtr<Module>>(ScriptOrModule, ModuleRequest const&)> host_resolve_imported_module;
+    Function<ThrowCompletionOr<NonnullGCPtr<Module>>(ScriptOrModule, ModuleRequest const&)> host_resolve_imported_module;
     Function<void(ScriptOrModule, ModuleRequest, PromiseCapability)> host_import_module_dynamically;
     Function<void(ScriptOrModule, ModuleRequest const&, PromiseCapability, Promise*)> host_finish_dynamic_import;
 
@@ -245,7 +245,7 @@ private:
     ThrowCompletionOr<void> property_binding_initialization(BindingPattern const& binding, Value value, Environment* environment);
     ThrowCompletionOr<void> iterator_binding_initialization(BindingPattern const& binding, Iterator& iterator_record, Environment* environment);
 
-    ThrowCompletionOr<NonnullRefPtr<Module>> resolve_imported_module(ScriptOrModule referencing_script_or_module, ModuleRequest const& module_request);
+    ThrowCompletionOr<NonnullGCPtr<Module>> resolve_imported_module(ScriptOrModule referencing_script_or_module, ModuleRequest const& module_request);
     ThrowCompletionOr<void> link_and_eval_module(Module& module);
 
     void import_module_dynamically(ScriptOrModule referencing_script_or_module, ModuleRequest module_request, PromiseCapability promise_capability);
@@ -275,7 +275,7 @@ private:
         ScriptOrModule referencing_script_or_module;
         String filename;
         String type;
-        NonnullRefPtr<Module> module;
+        Handle<Module> module;
         bool has_once_started_linking { false };
     };
 

+ 13 - 4
Userland/Libraries/LibJS/Script.cpp

@@ -13,7 +13,7 @@
 namespace JS {
 
 // 16.1.5 ParseScript ( sourceText, realm, hostDefined ), https://tc39.es/ecma262/#sec-parse-script
-Result<NonnullRefPtr<Script>, Vector<Parser::Error>> Script::parse(StringView source_text, Realm& realm, StringView filename, HostDefined* host_defined, size_t line_number_offset)
+Result<NonnullGCPtr<Script>, Vector<Parser::Error>> Script::parse(StringView source_text, Realm& realm, StringView filename, HostDefined* host_defined, size_t line_number_offset)
 {
     // 1. Let script be ParseText(sourceText, Script).
     auto parser = Parser(Lexer(source_text, filename, line_number_offset));
@@ -24,16 +24,25 @@ Result<NonnullRefPtr<Script>, Vector<Parser::Error>> Script::parse(StringView so
         return parser.errors();
 
     // 3. Return Script Record { [[Realm]]: realm, [[ECMAScriptCode]]: script, [[HostDefined]]: hostDefined }.
-    return adopt_ref(*new Script(realm, filename, move(script), host_defined));
+    return NonnullGCPtr(*realm.heap().allocate_without_realm<Script>(realm, filename, move(script), host_defined));
 }
 
 Script::Script(Realm& realm, StringView filename, NonnullRefPtr<Program> parse_node, HostDefined* host_defined)
-    : m_vm(realm.vm())
-    , m_realm(make_handle(&realm))
+    : m_realm(realm)
     , m_parse_node(move(parse_node))
     , m_filename(filename)
     , m_host_defined(host_defined)
 {
 }
 
+Script::~Script()
+{
+}
+
+void Script::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_realm);
+}
+
 }

+ 10 - 10
Userland/Libraries/LibJS/Script.h

@@ -7,8 +7,8 @@
 #pragma once
 
 #include <AK/NonnullRefPtr.h>
-#include <AK/RefCounted.h>
 #include <LibJS/AST.h>
+#include <LibJS/Heap/GCPtr.h>
 #include <LibJS/Heap/Handle.h>
 #include <LibJS/Parser.h>
 #include <LibJS/Runtime/Realm.h>
@@ -16,18 +16,18 @@
 namespace JS {
 
 // 16.1.4 Script Records, https://tc39.es/ecma262/#sec-script-records
-class Script
-    : public RefCounted<Script>
-    , public Weakable<Script> {
+class Script final : public Cell {
+    JS_CELL(Script, Cell);
+
 public:
     struct HostDefined {
         virtual ~HostDefined() = default;
     };
 
-    ~Script() = default;
-    static Result<NonnullRefPtr<Script>, Vector<Parser::Error>> parse(StringView source_text, Realm&, StringView filename = {}, HostDefined* = nullptr, size_t line_number_offset = 1);
+    virtual ~Script() override;
+    static Result<NonnullGCPtr<Script>, Vector<Parser::Error>> parse(StringView source_text, Realm&, StringView filename = {}, HostDefined* = nullptr, size_t line_number_offset = 1);
 
-    Realm& realm() { return *m_realm.cell(); }
+    Realm& realm() { return *m_realm; }
     Program const& parse_node() const { return *m_parse_node; }
 
     HostDefined* host_defined() { return m_host_defined; }
@@ -35,10 +35,10 @@ public:
 
 private:
     Script(Realm&, StringView filename, NonnullRefPtr<Program>, HostDefined* = nullptr);
-    // Handles are not safe unless we keep the VM alive.
-    NonnullRefPtr<VM> m_vm;
 
-    Handle<Realm> m_realm;               // [[Realm]]
+    virtual void visit_edges(Cell::Visitor&) override;
+
+    GCPtr<Realm> m_realm;                // [[Realm]]
     NonnullRefPtr<Program> m_parse_node; // [[ECMAScriptCode]]
 
     // Needed for potential lookups of modules.

+ 24 - 8
Userland/Libraries/LibJS/SourceTextModule.cpp

@@ -113,8 +113,14 @@ SourceTextModule::SourceTextModule(Realm& realm, StringView filename, bool has_t
 {
 }
 
+void SourceTextModule::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_import_meta);
+}
+
 // 16.2.1.6.1 ParseModule ( sourceText, realm, hostDefined ), https://tc39.es/ecma262/#sec-parsemodule
-Result<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> SourceTextModule::parse(StringView source_text, Realm& realm, StringView filename)
+Result<NonnullGCPtr<SourceTextModule>, Vector<Parser::Error>> SourceTextModule::parse(StringView source_text, Realm& realm, StringView filename)
 {
     // 1. Let body be ParseText(sourceText, Module).
     auto parser = Parser(Lexer(source_text, filename), Program::Type::Module);
@@ -239,7 +245,17 @@ Result<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> SourceTextModule:
     //          [[RequestedModules]]: requestedModules, [[ImportEntries]]: importEntries, [[LocalExportEntries]]: localExportEntries,
     //          [[IndirectExportEntries]]: indirectExportEntries, [[StarExportEntries]]: starExportEntries, [[DFSIndex]]: empty, [[DFSAncestorIndex]]: empty }.
     // FIXME: Add HostDefined
-    return adopt_ref(*new SourceTextModule(realm, filename, async, move(body), move(requested_modules), move(import_entries), move(local_export_entries), move(indirect_export_entries), move(star_export_entries), move(default_export)));
+    return NonnullGCPtr(*realm.heap().allocate_without_realm<SourceTextModule>(
+        realm,
+        filename,
+        async,
+        move(body),
+        move(requested_modules),
+        move(import_entries),
+        move(local_export_entries),
+        move(indirect_export_entries),
+        move(star_export_entries),
+        move(default_export)));
 }
 
 // 16.2.1.6.2 GetExportedNames ( [ exportStarSet ] ), https://tc39.es/ecma262/#sec-getexportednames
@@ -285,7 +301,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->make_weak_ptr(), entry.module_request()));
+        auto requested_module = TRY(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), entry.module_request()));
 
         // b. Let starNames be ? requestedModule.GetExportedNames(exportStarSet).
         auto star_names = TRY(requested_module->get_exported_names(vm, export_star_set));
@@ -339,7 +355,7 @@ ThrowCompletionOr<void> 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->make_weak_ptr(), import_entry.module_request()));
+        auto imported_module = MUST(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), 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
@@ -393,7 +409,7 @@ ThrowCompletionOr<void> 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->make_weak_ptr();
+    m_execution_context.script_or_module = NonnullGCPtr<Module>(*this);
 
     // 13. Set the VariableEnvironment of moduleContext to module.[[Environment]].
     m_execution_context.variable_environment = environment;
@@ -544,7 +560,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->make_weak_ptr(), entry.module_request()));
+        auto imported_module = TRY(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), entry.module_request()));
 
         // ii. If e.[[ImportName]] is all, then
         if (entry.kind == ExportStatement::ExportEntry::Kind::ModuleRequestAll) {
@@ -584,7 +600,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->make_weak_ptr(), entry.module_request()));
+        auto imported_module = TRY(vm.host_resolve_imported_module(NonnullGCPtr<Module>(*this), entry.module_request()));
 
         // b. Let resolution be ? importedModule.ResolveExport(exportName, resolveSet).
         auto resolution = TRY(imported_module->resolve_export(vm, export_name, resolve_set));
@@ -646,7 +662,7 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, Optional<Promis
     module_context.realm = &realm();
 
     // 4. Set the ScriptOrModule of moduleContext to module.
-    module_context.script_or_module = this->make_weak_ptr();
+    module_context.script_or_module = NonnullGCPtr<Module>(*this);
 
     // 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());

+ 8 - 13
Userland/Libraries/LibJS/SourceTextModule.h

@@ -16,28 +16,21 @@ namespace JS {
 
 // 16.2.1.6 Source Text Module Records, https://tc39.es/ecma262/#sec-source-text-module-records
 class SourceTextModule final : public CyclicModule {
+    JS_CELL(SourceTextModule, CyclicModule);
+
 public:
     using ImportEntry = ImportStatement::ImportEntry;
     using ExportEntry = ExportStatement::ExportEntry;
 
-    static Result<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> parse(StringView source_text, Realm&, StringView filename = {});
+    static Result<NonnullGCPtr<SourceTextModule>, Vector<Parser::Error>> parse(StringView source_text, Realm&, StringView filename = {});
 
     Program const& parse_node() const { return *m_ecmascript_code; }
 
     virtual ThrowCompletionOr<Vector<FlyString>> get_exported_names(VM& vm, Vector<Module*> export_star_set) override;
     virtual ThrowCompletionOr<ResolvedBinding> resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set = {}) override;
 
-    Object* import_meta()
-    {
-        if (m_import_meta.is_null())
-            return nullptr;
-        return m_import_meta.cell();
-    }
-
-    void set_import_meta(Badge<MetaProperty>, Object* import_meta)
-    {
-        m_import_meta = make_handle(import_meta);
-    }
+    Object* import_meta() { return m_import_meta; }
+    void set_import_meta(Badge<MetaProperty>, Object* import_meta) { m_import_meta = import_meta; }
 
 protected:
     virtual ThrowCompletionOr<void> initialize_environment(VM& vm) override;
@@ -49,9 +42,11 @@ private:
         Vector<ExportEntry> indirect_export_entries, Vector<ExportEntry> star_export_entries,
         RefPtr<ExportStatement> default_export);
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
     NonnullRefPtr<Program> m_ecmascript_code;      // [[ECMAScriptCode]]
     ExecutionContext m_execution_context;          // [[Context]]
-    Handle<Object> m_import_meta;                  // [[ImportMeta]]
+    GCPtr<Object> m_import_meta;                   // [[ImportMeta]]
     Vector<ImportEntry> m_import_entries;          // [[ImportEntries]]
     Vector<ExportEntry> m_local_export_entries;    // [[LocalExportEntries]]
     Vector<ExportEntry> m_indirect_export_entries; // [[IndirectExportEntries]]

+ 4 - 4
Userland/Libraries/LibJS/SyntheticModule.cpp

@@ -86,7 +86,7 @@ ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
     module_context.realm = &realm();
 
     // 5. Set the ScriptOrModule of moduleContext to module.
-    module_context.script_or_module = this->make_weak_ptr();
+    module_context.script_or_module = NonnullGCPtr<Module>(*this);
 
     // 6. Set the VariableEnvironment of moduleContext to module.[[Environment]].
     module_context.variable_environment = environment();
@@ -129,7 +129,7 @@ ThrowCompletionOr<void> SyntheticModule::set_synthetic_module_export(FlyString c
 }
 
 // 1.3 CreateDefaultExportSyntheticModule ( defaultExport ), https://tc39.es/proposal-json-modules/#sec-create-default-export-synthetic-module
-NonnullRefPtr<SyntheticModule> SyntheticModule::create_default_export_synthetic_module(Value default_export, Realm& realm, StringView filename)
+NonnullGCPtr<SyntheticModule> SyntheticModule::create_default_export_synthetic_module(Value default_export, Realm& realm, StringView filename)
 {
     // Note: Has some changes from PR: https://github.com/tc39/proposal-json-modules/pull/13.
     // 1. Let closure be the a Abstract Closure with parameters (module) that captures defaultExport and performs the following steps when called:
@@ -139,11 +139,11 @@ NonnullRefPtr<SyntheticModule> SyntheticModule::create_default_export_synthetic_
     };
 
     // 2. Return CreateSyntheticModule("default", closure, realm)
-    return adopt_ref(*new SyntheticModule({ "default" }, move(closure), realm, filename));
+    return *realm.heap().allocate_without_realm<SyntheticModule>(Vector<FlyString> { "default" }, move(closure), realm, filename);
 }
 
 // 1.4 ParseJSONModule ( source ), https://tc39.es/proposal-json-modules/#sec-parse-json-module
-ThrowCompletionOr<NonnullRefPtr<Module>> parse_json_module(StringView source_text, Realm& realm, StringView filename)
+ThrowCompletionOr<NonnullGCPtr<Module>> parse_json_module(StringView source_text, Realm& realm, StringView filename)
 {
     auto& vm = realm.vm();
 

+ 6 - 4
Userland/Libraries/LibJS/SyntheticModule.h

@@ -12,12 +12,12 @@ namespace JS {
 
 // 1.2 Synthetic Module Records, https://tc39.es/proposal-json-modules/#sec-synthetic-module-records
 class SyntheticModule final : public Module {
+    JS_CELL(SyntheticModule, Module);
+
 public:
     using EvaluationFunction = Function<ThrowCompletionOr<void>(SyntheticModule&)>;
 
-    SyntheticModule(Vector<FlyString> export_names, EvaluationFunction evaluation_steps, Realm& realm, StringView filename);
-
-    static NonnullRefPtr<SyntheticModule> create_default_export_synthetic_module(Value default_export, Realm& realm, StringView filename);
+    static NonnullGCPtr<SyntheticModule> create_default_export_synthetic_module(Value default_export, Realm& realm, StringView filename);
 
     ThrowCompletionOr<void> set_synthetic_module_export(FlyString const& export_name, Value export_value);
 
@@ -27,10 +27,12 @@ public:
     virtual ThrowCompletionOr<ResolvedBinding> resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set) override;
 
 private:
+    SyntheticModule(Vector<FlyString> export_names, EvaluationFunction evaluation_steps, Realm& realm, StringView filename);
+
     Vector<FlyString> m_export_names;      // [[ExportNames]]
     EvaluationFunction m_evaluation_steps; // [[EvaluationSteps]]
 };
 
-ThrowCompletionOr<NonnullRefPtr<Module>> parse_json_module(StringView source_text, Realm& realm, StringView filename);
+ThrowCompletionOr<NonnullGCPtr<Module>> parse_json_module(StringView source_text, Realm& realm, StringView filename);
 
 }

+ 2 - 2
Userland/Libraries/LibTest/JavaScriptTestRunner.h

@@ -231,7 +231,7 @@ inline ByteBuffer load_entire_file(StringView path)
     return buffer_or_error.release_value();
 }
 
-inline AK::Result<NonnullRefPtr<JS::Script>, ParserError> parse_script(StringView path, JS::Realm& realm)
+inline AK::Result<JS::NonnullGCPtr<JS::Script>, ParserError> parse_script(StringView path, JS::Realm& realm)
 {
     auto contents = load_entire_file(path);
     auto script_or_errors = JS::Script::parse(contents, realm, path);
@@ -244,7 +244,7 @@ inline AK::Result<NonnullRefPtr<JS::Script>, ParserError> parse_script(StringVie
     return script_or_errors.release_value();
 }
 
-inline AK::Result<NonnullRefPtr<JS::SourceTextModule>, ParserError> parse_module(StringView path, JS::Realm& realm)
+inline AK::Result<JS::NonnullGCPtr<JS::SourceTextModule>, ParserError> parse_module(StringView path, JS::Realm& realm)
 {
     auto contents = load_entire_file(path);
     auto script_or_errors = JS::SourceTextModule::parse(contents, realm, path);

+ 7 - 8
Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp

@@ -35,12 +35,12 @@ HTML::ClassicScript* active_script()
         return nullptr;
 
     // 3. Return record.[[HostDefined]].
-    if (record.has<WeakPtr<JS::Module>>()) {
+    if (record.has<JS::NonnullGCPtr<JS::Module>>()) {
         // FIXME: We don't currently have a module script.
         TODO();
     }
 
-    auto js_script = record.get<WeakPtr<JS::Script>>();
+    auto js_script = record.get<JS::NonnullGCPtr<JS::Script>>();
     VERIFY(js_script);
     VERIFY(js_script->host_defined());
     return verify_cast<HTML::ClassicScript>(js_script->host_defined());
@@ -76,10 +76,10 @@ JS::VM& main_thread_vm()
             //    The running script is the script in the [[HostDefined]] field in the ScriptOrModule component of the running JavaScript execution context.
             HTML::Script* script { nullptr };
             vm->running_execution_context().script_or_module.visit(
-                [&script](WeakPtr<JS::Script>& js_script) {
+                [&script](JS::NonnullGCPtr<JS::Script>& js_script) {
                     script = verify_cast<HTML::ClassicScript>(js_script->host_defined());
                 },
-                [](WeakPtr<JS::Module>&) {
+                [](JS::NonnullGCPtr<JS::Module>&) {
                     TODO();
                 },
                 [](Empty) {
@@ -240,8 +240,7 @@ JS::VM& main_thread_vm()
                     //        Since this requires pushing an execution context onto the stack, it also requires a global object. The only thing we can get a global object from in this case is the script or module.
                     //        To do this, we must assume script or module is not Empty. We must also assume that it is a Script Record for now as we don't currently run modules.
                     //        Do note that the JS spec gives _no_ guarantee that the execution context stack has something on it if HostEnqueuePromiseJob was called with a null realm: https://tc39.es/ecma262/#job-preparedtoevaluatecode
-                    VERIFY(script_or_module.has<WeakPtr<JS::Script>>());
-                    auto script_record = script_or_module.get<WeakPtr<JS::Script>>();
+                    VERIFY(script_or_module.has<JS::NonnullGCPtr<JS::Script>>());
                     dummy_execution_context = JS::ExecutionContext { vm->heap() };
                     dummy_execution_context->script_or_module = script_or_module;
                     vm->push_execution_context(dummy_execution_context.value());
@@ -285,7 +284,7 @@ JS::VM& main_thread_vm()
                 script_execution_context->function = nullptr;
                 script_execution_context->realm = &script->settings_object().realm();
                 VERIFY(script->script_record());
-                script_execution_context->script_or_module = script->script_record()->make_weak_ptr();
+                script_execution_context->script_or_module = JS::NonnullGCPtr<JS::Script>(*script->script_record());
             }
 
             // 5. Return the JobCallback Record { [[Callback]]: callable, [[HostDefined]]: { [[IncumbentSettings]]: incumbent settings, [[ActiveScriptContext]]: script execution context } }.
@@ -298,7 +297,7 @@ JS::VM& main_thread_vm()
         // FIXME: Implement 8.1.5.5.3 HostResolveImportedModule(referencingScriptOrModule, moduleRequest), https://html.spec.whatwg.org/multipage/webappapis.html#hostresolveimportedmodule(referencingscriptormodule,-modulerequest)
         // FIXME: Implement 8.1.5.5.4 HostGetSupportedImportAssertions(), https://html.spec.whatwg.org/multipage/webappapis.html#hostgetsupportedimportassertions
 
-        vm->host_resolve_imported_module = [](JS::ScriptOrModule, JS::ModuleRequest const&) -> JS::ThrowCompletionOr<NonnullRefPtr<JS::Module>> {
+        vm->host_resolve_imported_module = [](JS::ScriptOrModule, JS::ModuleRequest const&) -> JS::ThrowCompletionOr<JS::NonnullGCPtr<JS::Module>> {
             return vm->throw_completion<JS::InternalError>(JS::ErrorType::NotImplemented, "Modules in the browser");
         };