From 25909dcc051db82bf7ab4e554a8eb8ec8b173863 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 4 Oct 2022 21:00:08 +0100 Subject: [PATCH] LibWeb: Prepare to run callback in host_enqueue_promise_job() ...and clean up afterwards, of course. Additionally to preparing to run a script, we also prepare to run a callback here. This matches WebIDL's invoke_callback() / call_user_object_operation() functions, and prevents a crash in host_make_job_callback() when getting the incumbent settings object. Running the following JS no longer crashes after this change: ```js new Promise((resolve) => { setTimeout(resolve, 0); }).then(() => { return Promise.reject(); }); ``` See further discussion/investigation here: https://discord.com/channels/830522505605283862/830525031720943627/995019647214694511 https://discord.com/channels/830522505605283862/830525031720943627/1026824624358576158 https://discord.com/channels/830522505605283862/830525031720943627/1026922985581457458 Many thanks to Luke for doing the hard work here, tracking this down, and suggesting the fix! Co-authored-by: Luke Wilde --- Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index 684a92e2e1..8d15543ac4 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -234,6 +234,11 @@ JS::VM& main_thread_vm() // 2. If job settings is not null, then prepare to run script with job settings. job_settings->prepare_to_run_script(); + // IMPLEMENTATION DEFINED: Additionally to preparing to run a script, we also prepare to run a callback here. This matches WebIDL's + // invoke_callback() / call_user_object_operation() functions, and prevents a crash in host_make_job_callback() + // when getting the incumbent settings object. + job_settings->prepare_to_run_callback(); + // IMPLEMENTATION DEFINED: Per the previous "implementation defined" comment, we must now make the script or module the active script or module. // Since the only active execution context currently is the realm execution context of job settings, lets attach it here. job_settings->realm_execution_context().script_or_module = script_or_module; @@ -257,6 +262,9 @@ JS::VM& main_thread_vm() // IMPLEMENTATION DEFINED: Disassociate the realm execution context from the script or module. job_settings->realm_execution_context().script_or_module = Empty {}; + // IMPLEMENTATION DEFINED: See comment above, we need to clean up the non-standard prepare_to_run_callback() call. + job_settings->clean_up_after_running_callback(); + job_settings->clean_up_after_running_script(); } else { // Pop off the dummy execution context. See the above FIXME block about why this is done.