LibJS: Make PromiseJob store callback as a HeapFunction

This is a speculative fix for a flake seen on CI where a JobCallback
captured by a PromiseJob callback was GC'd prematurely.

Userland/Libraries/LibJS/Forward.h

@@ -296,6 +296,9 @@ struct TimeZoneMethods;
 struct PartialDurationRecord;
 };
 
+template<typename T>
+class HeapFunction;
+
 template<typename T>
 requires(!IsLvalueReference<T>)
 class ThrowCompletionOr;

Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp

@@ -90,9 +90,9 @@ PromiseJob create_promise_reaction_job(VM& vm, PromiseReaction& reaction, Value
 {
     // 1. Let job be a new Job Abstract Closure with no parameters that captures reaction and argument and performs the following steps when called:
     //    See run_reaction_job for "the following steps".
-    auto job = [&vm, reaction = make_handle(&reaction), argument = make_handle(argument)] {
+    auto job = create_heap_function(vm.heap(), [&vm, reaction = make_handle(&reaction), argument = make_handle(argument)] {
         return run_reaction_job(vm, *reaction.cell(), argument.value());
-    };
+    });
 
     // 2. Let handlerRealm be null.
     Realm* handler_realm { nullptr };
@@ -115,7 +115,7 @@ PromiseJob create_promise_reaction_job(VM& vm, PromiseReaction& reaction, Value
     }
 
     // 4. Return the Record { [[Job]]: job, [[Realm]]: handlerRealm }.
-    return { move(job), handler_realm };
+    return { job, handler_realm };
 }
 
 // 27.2.2.2 NewPromiseResolveThenableJob ( promiseToResolve, thenable, then ), https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob
@@ -164,12 +164,12 @@ PromiseJob create_promise_resolve_thenable_job(VM& vm, Promise& promise_to_resol
 
     // 1. Let job be a new Job Abstract Closure with no parameters that captures promiseToResolve, thenable, and then and performs the following steps when called:
     //    See run_resolve_thenable_job() for "the following steps".
-    auto job = [&vm, promise_to_resolve = make_handle(promise_to_resolve), thenable = make_handle(thenable), then]() mutable {
+    auto job = create_heap_function(vm.heap(), [&vm, promise_to_resolve = make_handle(promise_to_resolve), thenable = make_handle(thenable), then]() mutable {
         return run_resolve_thenable_job(vm, *promise_to_resolve.cell(), thenable.value(), then);
-    };
+    });
 
     // 6. Return the Record { [[Job]]: job, [[Realm]]: thenRealm }.
-    return { move(job), then_realm };
+    return { job, then_realm };
 }
 
 }

Userland/Libraries/LibJS/Runtime/PromiseJobs.h

@@ -15,7 +15,7 @@
 namespace JS {
 
 struct PromiseJob {
-    Function<ThrowCompletionOr<Value>()> job;
+    NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>> job;
     GCPtr<Realm> realm;
 };
 

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -86,8 +86,8 @@ VM::VM(OwnPtr<CustomData> custom_data, ErrorMessages error_messages)
         enqueue_finalization_registry_cleanup_job(finalization_registry);
     };
 
-    host_enqueue_promise_job = [this](Function<ThrowCompletionOr<Value>()> job, Realm* realm) {
-        enqueue_promise_job(move(job), realm);
+    host_enqueue_promise_job = [this](NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>> job, Realm* realm) {
+        enqueue_promise_job(job, realm);
     };
 
     host_make_job_callback = [](FunctionObject& function_object) {
@@ -214,6 +214,9 @@ void VM::gather_roots(HashMap<Cell*, HeapRoot>& roots)
     gather_roots_from_execution_context_stack(m_execution_context_stack);
     for (auto& saved_stack : m_saved_execution_context_stacks)
         gather_roots_from_execution_context_stack(saved_stack);
+
+    for (auto& job : m_promise_jobs)
+        roots.set(job, HeapRoot { .type = HeapRoot::Type::VM });
 }
 
 ThrowCompletionOr<Value> VM::named_evaluation_if_anonymous_function(ASTNode const& expression, DeprecatedFlyString const& name)
@@ -636,19 +639,19 @@ void VM::run_queued_promise_jobs()
         auto job = m_promise_jobs.take_first();
         dbgln_if(PROMISE_DEBUG, "Calling promise job function");
 
-        [[maybe_unused]] auto result = job();
+        [[maybe_unused]] auto result = job->function()();
     }
 }
 
 // 9.5.4 HostEnqueuePromiseJob ( job, realm ), https://tc39.es/ecma262/#sec-hostenqueuepromisejob
-void VM::enqueue_promise_job(Function<ThrowCompletionOr<Value>()> job, Realm*)
+void VM::enqueue_promise_job(NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>> job, Realm*)
 {
     // An implementation of HostEnqueuePromiseJob must conform to the requirements in 9.5 as well as the following:
     // - FIXME: If realm is not null, each time job is invoked the implementation must perform implementation-defined steps such that execution is prepared to evaluate ECMAScript code at the time of job's invocation.
     // - FIXME: Let scriptOrModule be GetActiveScriptOrModule() at the time HostEnqueuePromiseJob is invoked. If realm is not null, each time job is invoked the implementation must perform implementation-defined steps
     //          such that scriptOrModule is the active script or module at the time of job's invocation.
     // - Jobs must run in the same order as the HostEnqueuePromiseJob invocations that scheduled them.
-    m_promise_jobs.append(move(job));
+    m_promise_jobs.append(job);
 }
 
 void VM::run_queued_finalization_registry_cleanup_jobs()

Userland/Libraries/LibJS/Runtime/VM.h

@@ -210,7 +210,7 @@ public:
     CommonPropertyNames names;
 
     void run_queued_promise_jobs();
-    void enqueue_promise_job(Function<ThrowCompletionOr<Value>()> job, Realm*);
+    void enqueue_promise_job(NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>> job, Realm*);
 
     void run_queued_finalization_registry_cleanup_jobs();
     void enqueue_finalization_registry_cleanup_job(FinalizationRegistry&);
@@ -254,7 +254,7 @@ public:
     Function<void(Promise&, Promise::RejectionOperation)> host_promise_rejection_tracker;
     Function<ThrowCompletionOr<Value>(JobCallback&, Value, ReadonlySpan<Value>)> host_call_job_callback;
     Function<void(FinalizationRegistry&)> host_enqueue_finalization_registry_cleanup_job;
-    Function<void(Function<ThrowCompletionOr<Value>()>, Realm*)> host_enqueue_promise_job;
+    Function<void(NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>>, Realm*)> host_enqueue_promise_job;
     Function<JS::NonnullGCPtr<JobCallback>(FunctionObject&)> host_make_job_callback;
     Function<ThrowCompletionOr<void>(Realm&)> host_ensure_can_compile_strings;
     Function<ThrowCompletionOr<void>(Object&)> host_ensure_can_add_private_element;
@@ -300,7 +300,7 @@ private:
     // GlobalSymbolRegistry, https://tc39.es/ecma262/#table-globalsymbolregistry-record-fields
     HashMap<String, NonnullGCPtr<Symbol>> m_global_symbol_registry;
 
-    Vector<Function<ThrowCompletionOr<Value>()>> m_promise_jobs;
+    Vector<NonnullGCPtr<HeapFunction<ThrowCompletionOr<Value>()>>> m_promise_jobs;
 
     Vector<GCPtr<FinalizationRegistry>> m_finalization_registry_cleanup_jobs;
 

Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp

@@ -249,7 +249,7 @@ ErrorOr<void> initialize_main_thread_vm()
     };
 
     // 8.1.5.4.3 HostEnqueuePromiseJob(job, realm), https://html.spec.whatwg.org/multipage/webappapis.html#hostenqueuepromisejob
-    s_main_thread_vm->host_enqueue_promise_job = [](Function<JS::ThrowCompletionOr<JS::Value>()> job, JS::Realm* realm) {
+    s_main_thread_vm->host_enqueue_promise_job = [](JS::NonnullGCPtr<JS::HeapFunction<JS::ThrowCompletionOr<JS::Value>()>> job, JS::Realm* realm) {
         // 1. If realm is not null, then let job settings be the settings object for realm. Otherwise, let job settings be null.
         HTML::EnvironmentSettingsObject* job_settings { nullptr };
         if (realm)
@@ -296,7 +296,7 @@ ErrorOr<void> initialize_main_thread_vm()
             }
 
             // 3. Let result be job().
-            [[maybe_unused]] auto result = job();
+            auto result = job->function()();
 
             // 4. If job settings is not null, then clean up after running script with job settings.
             if (job_settings) {