From 48a09512b897591b71af84cccdc09b88316e0127 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Sun, 15 Mar 2026 00:50:40 -0600 Subject: [PATCH 1/6] Implement FinalizationRegistry --- Cargo.lock | 1 + core/engine/Cargo.toml | 1 + .../src/builtins/finalization_registry/mod.rs | 389 ++++++++++++++++++ core/engine/src/builtins/mod.rs | 3 + core/engine/src/builtins/promise/mod.rs | 4 +- core/engine/src/context/hooks.rs | 2 +- core/engine/src/context/intrinsics.rs | 14 + core/engine/src/job.rs | 46 ++- .../builtins/jsfinalization_registry.rs | 108 +++++ core/engine/src/object/jsobject.rs | 4 + core/string/src/common.rs | 2 + test262_config.toml | 1 - 12 files changed, 570 insertions(+), 5 deletions(-) create mode 100644 core/engine/src/builtins/finalization_registry/mod.rs create mode 100644 core/engine/src/object/builtins/jsfinalization_registry.rs diff --git a/Cargo.lock b/Cargo.lock index 5897615ab05..5e5bff607d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -379,6 +379,7 @@ version = "1.0.0-dev" dependencies = [ "aligned-vec", "arrayvec", + "async-channel", "bitflags", "boa_ast", "boa_gc", diff --git a/core/engine/Cargo.toml b/core/engine/Cargo.toml index 480a7a7adeb..8a070e5ecea 100644 --- a/core/engine/Cargo.toml +++ b/core/engine/Cargo.toml @@ -141,6 +141,7 @@ aligned-vec.workspace = true dynify = { workspace = true, features = ["macros"] } futures-concurrency.workspace = true oneshot = { workspace = true, features = ["async"] } +async-channel.workspace = true # intl deps boa_icu_provider = { workspace = true, features = ["std"], optional = true } diff --git a/core/engine/src/builtins/finalization_registry/mod.rs b/core/engine/src/builtins/finalization_registry/mod.rs new file mode 100644 index 00000000000..b8ceb36aba7 --- /dev/null +++ b/core/engine/src/builtins/finalization_registry/mod.rs @@ -0,0 +1,389 @@ +//! Boa's implementation of ECMAScript's `FinalizationRegistry` object. + +use std::{ + cell::{Cell, RefCell}, + slice, +}; + +use boa_gc::{Ephemeron, Finalize, Gc, Trace, WeakGc}; + +use crate::{ + Context, JsArgs, JsData, JsObject, JsResult, JsSymbol, JsValue, JsVariant, + context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, + job::{Job, JobCallback, NativeAsyncJob}, + js_error, js_string, + object::{ + ErasedVTableObject, JsFunction, VTableObject, + internal_methods::get_prototype_from_constructor, + }, + property::Attribute, + realm::Realm, + string::StaticJsStrings, +}; + +use super::{BuiltInConstructor, BuiltInObject, IntrinsicObject, builder::BuiltInBuilder}; + +/// On GG collection, sends a message to a [`FinalizationRegistry`] indicating that it needs to +/// be collected. +#[derive(Trace)] +#[boa_gc(unsafe_empty_trace)] +struct CleanupSignaler(Cell>>); + +impl Finalize for CleanupSignaler { + fn finalize(&self) { + if let Some(sender) = self.0.take() + && let Some(sender) = sender.upgrade() + { + // We don't need to handle errors: + // - If the channel is full, the `FinalizationRegistry` has already + // been enqueued for cleanup. + // - If the channel is closed, the `FinalizationRegistry` was + // GC'd, so we don't need to worry about cleanups. + let _ = sender.try_send(()); + } + } +} + +/// A cell tracked by a [`FinalizationRegistry`]. +#[derive(Trace, Finalize)] +pub(crate) struct RegistryCell { + target: Ephemeron, + held_value: JsValue, + unregister_token: Option>, +} + +/// Boa's implementation of ECMAScript's [`FinalizationRegistry`] builtin object. +/// +/// `FinalizationRegistry` provides a way to request that a cleanup callback get called at some point +/// when a value registered with the registry has been reclaimed (garbage-collected). +/// +/// [`FinalizationRegistry`]: https://tc39.es/ecma262/#sec-finalization-registry-objects +#[derive(Trace, Finalize, JsData)] +pub(crate) struct FinalizationRegistry { + realm: Realm, + callback: JobCallback, + #[unsafe_ignore_trace] + cleanup_notifier: async_channel::Sender<()>, + cells: Vec, +} + +impl IntrinsicObject for FinalizationRegistry { + fn get(intrinsics: &Intrinsics) -> JsObject { + Self::STANDARD_CONSTRUCTOR(intrinsics.constructors()).constructor() + } + + fn init(realm: &Realm) { + BuiltInBuilder::from_standard_constructor::(realm) + .property( + JsSymbol::to_string_tag(), + js_string!("FinalizationRegistry"), + Attribute::CONFIGURABLE, + ) + .method(Self::register, js_string!("register"), 2) + .method(Self::unregister, js_string!("unregister"), 1) + .build(); + } +} + +impl BuiltInObject for FinalizationRegistry { + const NAME: crate::JsString = StaticJsStrings::FINALIZATION_REGISTRY; + + const ATTRIBUTE: Attribute = Attribute::WRITABLE.union(Attribute::CONFIGURABLE); +} + +impl BuiltInConstructor for FinalizationRegistry { + const CONSTRUCTOR_ARGUMENTS: usize = 1; + const CONSTRUCTOR_STORAGE_SLOTS: usize = 0; + const PROTOTYPE_STORAGE_SLOTS: usize = 3; + + const STANDARD_CONSTRUCTOR: fn(&StandardConstructors) -> &StandardConstructor = + StandardConstructors::finalization_registry; + + /// Constructor [`FinalizationRegistry ( cleanupCallback )`][cons] + /// + /// [cons]: https://tc39.es/ecma262/#sec-finalization-registry-cleanup-callback + fn constructor( + new_target: &JsValue, + args: &[JsValue], + context: &mut Context, + ) -> JsResult { + // 1. If NewTarget is undefined, throw a TypeError exception. + if new_target.is_undefined() { + return Err(js_error!( + TypeError: "FinalizationRegistry: cannot call constructor without `new`" + )); + } + + // 2. If IsCallable(cleanupCallback) is false, throw a TypeError exception. + let callback = args + .get_or_undefined(0) + .as_object() + .and_then(JsFunction::from_object) + .ok_or_else(|| { + js_error!( + TypeError: "FinalizationRegistry: \ + cleanup callback of registry must be callable" + ) + })?; + + // 3. Let finalizationRegistry be ? OrdinaryCreateFromConstructor(NewTarget, + // "%FinalizationRegistry.prototype%", « [[Realm]], [[CleanupCallback]], [[Cells]] »). + let prototype = get_prototype_from_constructor( + new_target, + StandardConstructors::finalization_registry, + context, + )?; + + // 4. Let fn be the active function object. + // 5. Set finalizationRegistry.[[Realm]] to fn.[[Realm]]. + let realm = context.vm.frame().realm.clone(); + + // 6. Set finalizationRegistry.[[CleanupCallback]] to HostMakeJobCallback(cleanupCallback). + let callback = context.host_hooks().make_job_callback(callback, context); + + // 7. Set finalizationRegistry.[[Cells]] to a new empty List. + let cells = Vec::new(); + + let (sender, receiver) = async_channel::bounded(1); + + let registry = JsObject::new_unique( + prototype, + FinalizationRegistry { + realm, + callback, + cells, + cleanup_notifier: sender, + }, + ); + + let weak_registry = WeakGc::new(registry.inner()); + + { + async fn inner_cleanup( + weak_registry: WeakGc>, + receiver: async_channel::Receiver<()>, + context: &RefCell<&mut Context>, + ) -> JsResult { + let Ok(()) = receiver.recv().await else { + return Ok(JsValue::undefined()); + }; + + let Some(registry) = weak_registry.upgrade().map(JsObject::from_inner) else { + return Ok(JsValue::undefined()); + }; + + let result = FinalizationRegistry::cleanup(®istry, &mut context.borrow_mut()); + + context + .borrow_mut() + .enqueue_job(Job::FinalizationRegistryCleanupJob(NativeAsyncJob::new( + async move |context| inner_cleanup(weak_registry, receiver, context).await, + ))); + + result.map(|()| JsValue::undefined()) + } + + context.enqueue_job(Job::FinalizationRegistryCleanupJob(NativeAsyncJob::new( + async move |ctx| inner_cleanup(weak_registry, receiver, ctx).await, + ))); + } + + // 8. Return finalizationRegistry. + Ok(registry.upcast().into()) + } +} + +impl FinalizationRegistry { + /// [`FinalizationRegistry.prototype.register ( target, heldValue [ , unregisterToken ] )`][spec] + /// + /// [spec]: https://tc39.es/ecma262/sec-finalization-registry.prototype.register + fn register(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult { + // 1. Let finalizationRegistry be the this value. + // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]). + let this = this.as_object(); + let mut registry = this + .as_ref() + .and_then(JsObject::downcast_mut::) + .ok_or_else(|| { + js_error!( + TypeError: "FinalizationRegistry.prototype.register: \ + invalid object type for `this`", + ) + })?; + + let target = args.get_or_undefined(0); + let held_value = args.get_or_undefined(1); + let unregister_token = args.get_or_undefined(2); + + // 3. If CanBeHeldWeakly(target) is false, throw a TypeError exception. + // TODO: support Symbols + let Some(target_obj) = target.as_object() else { + return Err(js_error!( + TypeError: "FinalizationRegistry.prototype.register: \ + `target` must be an Object or Symbol", + )); + }; + + // 4. If SameValue(target, heldValue) is true, throw a TypeError exception. + if target == held_value { + return Err(js_error!( + TypeError: "FinalizationRegistry.prototype.register: \ + `heldValue` cannot be the same as `target`" + )); + } + + // 5. If CanBeHeldWeakly(unregisterToken) is false, then + // TODO: support Symbols + let unregister_token = match unregister_token.variant() { + JsVariant::Object(obj) => Some(WeakGc::new(obj.inner())), + // b. Set unregisterToken to empty. + JsVariant::Undefined => None, + // a. If unregisterToken is not undefined, throw a TypeError exception. + _ => { + return Err(js_error!( + TypeError: "FinalizationRegistry.prototype.register: \ + `unregisterToken` must be an Object, a Symbol, or undefined", + )); + } + }; + + // 6. Let cell be the Record { [[WeakRefTarget]]: target, [[HeldValue]]: heldValue, [[UnregisterToken]]: unregisterToken }. + let cell = RegistryCell { + target: Ephemeron::new( + target_obj.inner(), + CleanupSignaler(Cell::new(Some( + registry.cleanup_notifier.clone().downgrade(), + ))), + ), + held_value: held_value.clone(), + unregister_token, + }; + + // 7. Append cell to finalizationRegistry.[[Cells]]. + registry.cells.push(cell); + + // 8. Return undefined. + Ok(JsValue::undefined()) + } + + /// [`FinalizationRegistry.prototype.unregister ( unregisterToken )`][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-finalization-registry.prototype.unregister + fn unregister(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult { + // 1. Let finalizationRegistry be the this value. + // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]). + let this = this.as_object(); + let mut registry = this + .as_ref() + .and_then(JsObject::downcast_mut::) + .ok_or_else(|| { + js_error!( + TypeError: "FinalizationRegistry.prototype.register: \ + invalid object type for `this`", + ) + })?; + + // 3. If CanBeHeldWeakly(unregisterToken) is false, throw a TypeError exception.\ + // TODO: support Symbols + let unregister_token = args.get_or_undefined(0).as_object(); + let unregister_token = unregister_token + .as_ref() + .map(JsObject::inner) + .ok_or_else(|| { + js_error!( + TypeError: "FinalizationRegistry.prototype.unregister: \ + `unregisterToken` must be an Object or a Symbol.", + ) + })?; + + // 4. Let removed be false. + let mut removed = false; + let mut i = 0; + // 5. For each Record { [[WeakRefTarget]], [[HeldValue]], [[UnregisterToken]] } cell of finalizationRegistry.[[Cells]], do + while i < registry.cells.len() { + let cell = ®istry.cells[i]; + + // a. If cell.[[UnregisterToken]] is not empty and SameValue(cell.[[UnregisterToken]], unregisterToken) is true, then + if let Some(tok) = cell.unregister_token.as_ref() + && let Some(tok) = tok.upgrade() + && Gc::ptr_eq(&tok, unregister_token) + { + // i. Remove cell from finalizationRegistry.[[Cells]]. + let cell = registry.cells.swap_remove(i); + let _key = cell.target.key(); + // TODO: it might be better to add a special ref for the value that + // also preserves the original key instead. + // SAFETY: the original key is alive per our previous call to `key`, + // so if this returns `Some`, then the value cannot be collected + // until `key` gets dropped. + unsafe { + cell.target.value_ref().and_then(|v| v.0.take()); + } + + // ii. Set removed to true. + removed = true; + } else { + i += 1; + } + } + + // 6. Return removed. + Ok(removed.into()) + } + + /// Abstract operation [`CleanupFinalizationRegistry ( finalizationRegistry )`][spec]. + /// + /// Cleans up all the cells of the finalization registry that are determined to be + /// unreachable by the garbage collector. + /// + /// # Panics + /// + /// Panics if `obj` is not a `FinalizationRegistry` object. + /// + /// [spec]: https://tc39.es/ecma262/#sec-cleanup-finalization-registry + pub(crate) fn cleanup( + obj: &JsObject, + context: &mut Context, + ) -> JsResult<()> { + // 1. Assert: finalizationRegistry has [[Cells]] and [[CleanupCallback]] internal slots. + // let obj = obj.borrow_mut(); + // let registry = obj.data_mut(); + + // 2. Let callback be finalizationRegistry.[[CleanupCallback]]. + let callback = std::mem::replace( + &mut obj.borrow_mut().data_mut().callback, + JobCallback::new(context.intrinsics().objects().throw_type_error(), ()), + ); + + let mut i = 0; + let result = loop { + if i >= obj.borrow().data().cells.len() { + break Ok(()); + } + // 3. While finalizationRegistry.[[Cells]] contains a Record cell such that cell.[[WeakRefTarget]] is empty, an implementation may perform the following steps: + if obj.borrow().data().cells[i].target.has_value() { + i += 1; + } else { + // a. Choose any such cell. + // b. Remove cell from finalizationRegistry.[[Cells]]. + let cell = obj.borrow_mut().data_mut().cells.swap_remove(i); + // c. Perform ? HostCallJobCallback(callback, undefined, « cell.[[HeldValue]] »). + let result = context.host_hooks().call_job_callback( + &callback, + &JsValue::undefined(), + slice::from_ref(&cell.held_value), + context, + ); + + if let Err(err) = result { + break Err(err); + } + } + }; + + obj.borrow_mut().data_mut().callback = callback; + + // 4. Return unused. + result + } +} diff --git a/core/engine/src/builtins/mod.rs b/core/engine/src/builtins/mod.rs index 470e6ea399b..3f664c69866 100644 --- a/core/engine/src/builtins/mod.rs +++ b/core/engine/src/builtins/mod.rs @@ -12,6 +12,7 @@ pub mod dataview; pub mod date; pub mod error; pub mod eval; +pub mod finalization_registry; pub mod function; pub mod generator; pub mod generator_function; @@ -66,6 +67,7 @@ pub(crate) use self::{ AggregateError, EvalError, RangeError, ReferenceError, SyntaxError, TypeError, UriError, }, eval::Eval, + finalization_registry::FinalizationRegistry, function::BuiltInFunctionObject, json::Json, map::Map, @@ -316,6 +318,7 @@ impl Realm { WeakMap::init(self); WeakSet::init(self); Atomics::init(self); + FinalizationRegistry::init(self); #[cfg(feature = "annex-b")] { diff --git a/core/engine/src/builtins/promise/mod.rs b/core/engine/src/builtins/promise/mod.rs index e8d09fc3514..3893ce8eeaa 100644 --- a/core/engine/src/builtins/promise/mod.rs +++ b/core/engine/src/builtins/promise/mod.rs @@ -2629,7 +2629,7 @@ fn new_promise_reaction_job( }, // e. Else, let handlerResult be Completion(HostCallJobCallback(handler, undefined, « argument »)). Some(handler) => match context.host_hooks().call_job_callback( - handler, + &handler, &JsValue::undefined(), std::slice::from_ref(&argument), context, @@ -2707,7 +2707,7 @@ fn new_promise_resolve_thenable_job( // b. Let thenCallResult be Completion(HostCallJobCallback(then, thenable, « resolvingFunctions.[[Resolve]], resolvingFunctions.[[Reject]] »)). let then_call_result = context.host_hooks().call_job_callback( - then, + &then, &thenable, &[ resolving_functions.resolve.clone().into(), diff --git a/core/engine/src/context/hooks.rs b/core/engine/src/context/hooks.rs index 035389a115b..c1f2906563c 100644 --- a/core/engine/src/context/hooks.rs +++ b/core/engine/src/context/hooks.rs @@ -82,7 +82,7 @@ pub trait HostHooks { #[cfg_attr(feature = "native-backtrace", track_caller)] fn call_job_callback( &self, - job: JobCallback, + job: &JobCallback, this: &JsValue, args: &[JsValue], context: &mut Context, diff --git a/core/engine/src/context/intrinsics.rs b/core/engine/src/context/intrinsics.rs index b4dcc162e9d..98e601c9d4f 100644 --- a/core/engine/src/context/intrinsics.rs +++ b/core/engine/src/context/intrinsics.rs @@ -171,6 +171,7 @@ pub struct StandardConstructors { weak_map: StandardConstructor, weak_set: StandardConstructor, iterator: StandardConstructor, + finalization_registry: StandardConstructor, #[cfg(feature = "intl")] collator: StandardConstructor, #[cfg(feature = "intl")] @@ -266,6 +267,7 @@ impl Default for StandardConstructors { weak_map: StandardConstructor::default(), weak_set: StandardConstructor::default(), iterator: StandardConstructor::default(), + finalization_registry: StandardConstructor::default(), #[cfg(feature = "intl")] collator: StandardConstructor::default(), #[cfg(feature = "intl")] @@ -856,6 +858,18 @@ impl StandardConstructors { &self.iterator } + /// Returns the `FinalizationRegistry` constructor. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-finalization-registry-constructor + #[inline] + #[must_use] + pub const fn finalization_registry(&self) -> &StandardConstructor { + &self.finalization_registry + } + /// Returns the `Intl.Collator` constructor. /// /// More information: diff --git a/core/engine/src/job.rs b/core/engine/src/job.rs index 297a1e9ba72..46208b03384 100644 --- a/core/engine/src/job.rs +++ b/core/engine/src/job.rs @@ -535,6 +535,34 @@ pub enum Job { /// /// See [`GenericJob`] for more information. GenericJob(GenericJob), + /// A job that will eventually cleanup a `FinalizationRegistry`. + /// + /// This job differs slightly from the [spec]; originally it's defined + /// as being enqueued exactly when a `FinalizationRegistry` needs to call + /// `FinalizationRegistry::cleanup`, but here it's defined as an async + /// job that suspends execution until it receives a signal from the engine + /// that the `FinalizationRegistry` needs to be cleaned up. + /// + /// # Execution + /// + /// As described on the [spec's section about execution][execution], + /// + /// > Because calling HostEnqueueFinalizationRegistryCleanupJob is optional, + /// > registered objects in a FinalizationRegistry do not necessarily hold + /// > that FinalizationRegistry live. Implementations may omit FinalizationRegistry + /// > callbacks for any reason, e.g., if the FinalizationRegistry itself becomes + /// > dead, or if the application is shutting down. + /// + /// For this reason, it is recommended to exclude `FinalizationRegistry` cleanup + /// jobs from any condition that returns from [`JobExecutor::run_jobs`]. + /// + /// By the same token, it is recommended to execute [`FinalizationRegistryCleanubJob`] + /// separately from all other enqueued [`NativeAsyncJob`]s, prioritizing the + /// execution of all other jobs if possible. + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakref-host-hooks + /// [execution]: https://tc39.es/ecma262/#sec-weakref-execution + FinalizationRegistryCleanupJob(NativeAsyncJob), } impl From for Job { @@ -629,6 +657,7 @@ impl JobExecutor for IdleJobExecutor { pub struct SimpleJobExecutor { promise_jobs: RefCell>, async_jobs: RefCell>, + finalization_registry_jobs: RefCell>, timeout_jobs: RefCell>>, generic_jobs: RefCell>, stop: Arc, @@ -686,6 +715,9 @@ impl JobExecutor for SimpleJobExecutor { .push(t); } Job::GenericJob(g) => self.generic_jobs.borrow_mut().push_back(g), + Job::FinalizationRegistryCleanupJob(fr) => { + self.finalization_registry_jobs.borrow_mut().push_back(fr); + } } } @@ -698,6 +730,7 @@ impl JobExecutor for SimpleJobExecutor { Self: Sized, { let mut group = FutureGroup::new(); + let mut fr_group = FutureGroup::new(); loop { if self.stop.load(Ordering::Relaxed) { self.stop.store(false, Ordering::Relaxed); @@ -709,6 +742,10 @@ impl JobExecutor for SimpleJobExecutor { group.insert(job.call(context)); } + for job in mem::take(&mut *self.finalization_registry_jobs.borrow_mut()) { + fr_group.insert(job.call(context)); + } + // Dispatch all past-due timeout jobs before the termination check. { let now = context.borrow().clock().now(); @@ -735,7 +772,14 @@ impl JobExecutor for SimpleJobExecutor { } if self.is_empty() && group.is_empty() { - break; + match future::poll_once(fr_group.next()).await.flatten() { + Some(Err(err)) => { + self.clear(); + return Err(err); + } + _ if !self.is_empty() => {} + _ => break, + } } if let Some(Err(err)) = future::poll_once(group.next()).await.flatten() { diff --git a/core/engine/src/object/builtins/jsfinalization_registry.rs b/core/engine/src/object/builtins/jsfinalization_registry.rs new file mode 100644 index 00000000000..fff3e5b6ca3 --- /dev/null +++ b/core/engine/src/object/builtins/jsfinalization_registry.rs @@ -0,0 +1,108 @@ +use std::ops::Deref; + +use boa_gc::{Finalize, Trace}; + +use crate::{ + builtins::finalization_registry::FinalizationRegistry, object::JsObjectType, realm::Realm, + value::TryFromJs, Context, JsNativeError, JsObject, JsResult, JsValue, +}; + +/// `JsFinalizationRegistry` provides a wrapper for Boa's implementation of the ECMAScript +/// [`FinalizationRegistry`] object. +/// +/// [`FinalizationRegistry`]: https://tc39.es/ecma262/#sec-finalization-registry-objects +#[derive(Debug, Clone, Trace, Finalize)] +pub struct JsFinalizationRegistry { + inner: JsObject, +} + +impl JsFinalizationRegistry { + /// Creates a [`JsFinalizationRegistry`] from a [`JsObject`], erroring if the object is not + /// of the required kind. + #[inline] + pub fn from_object(object: JsObject) -> JsResult { + if object.is::() { + Ok(Self { inner: object }) + } else { + Err(JsNativeError::typ() + .with_message("object is not a TypedArray") + .into()) + } + } + + /// Gets the `[[Realm]]` slot of this [`FinalizationRegistry`]. + pub fn realm(&self) -> Realm { + self.downcast_ref::() + .expect("must be a `FinalizationRegistry") + .realm + .clone() + } + + /// Returns `true` if this finalization registry has unreachable cells. + pub(crate) fn needs_cleanup(&self) -> bool { + self.downcast_ref::() + .expect("must be a `FinalizationRegistry") + .needs_cleanup + .get() + } + + /// Clears the `needs_cleanup` flag from the registry. + pub(crate) fn clear_needs_cleanup(&self) { + self.downcast_ref::() + .expect("must be a `FinalizationRegistry") + .needs_cleanup + .set(false) + } + + /// Abstract operation [`CleanupFinalizationRegistry ( finalizationRegistry )`][spec]. + /// + /// Cleans up all the cells of the finalization registry that are determined to be + /// unreachable by the garbage collector. + /// + /// [spec]: https://tc39.es/ecma262/#sec-cleanup-finalization-registry + pub fn cleanup(&self, context: &mut Context) -> JsResult<()> { + FinalizationRegistry::cleanup(&self, context) + } + + /// Creates a new [`JsFinalizationRegistry`] from an object, without checking if the object is + /// a finalization registry. + pub(crate) fn from_object_unchecked(object: JsObject) -> Self { + Self { inner: object } + } +} + +impl From for JsObject { + #[inline] + fn from(o: JsFinalizationRegistry) -> Self { + o.inner.clone() + } +} + +impl From for JsValue { + #[inline] + fn from(o: JsFinalizationRegistry) -> Self { + o.inner.clone().into() + } +} + +impl Deref for JsFinalizationRegistry { + type Target = JsObject; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl JsObjectType for JsFinalizationRegistry {} + +impl TryFromJs for JsFinalizationRegistry { + fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult { + match value { + JsValue::Object(o) => Self::from_object(o.clone()), + _ => Err(JsNativeError::typ() + .with_message("value is not a FinalizationRegistry object") + .into()), + } + } +} diff --git a/core/engine/src/object/jsobject.rs b/core/engine/src/object/jsobject.rs index 0c41bc1d330..d755236d11f 100644 --- a/core/engine/src/object/jsobject.rs +++ b/core/engine/src/object/jsobject.rs @@ -1042,6 +1042,10 @@ impl JsObject { &self.inner } + pub(crate) fn from_inner(inner: Gc>) -> Self { + Self { inner } + } + /// Create a new private name with this object as the unique identifier. pub(crate) fn private_name(&self, description: JsString) -> PrivateName { let ptr: *const _ = self.as_ref(); diff --git a/core/string/src/common.rs b/core/string/src/common.rs index 993fff83edc..132c1c97afe 100644 --- a/core/string/src/common.rs +++ b/core/string/src/common.rs @@ -170,6 +170,7 @@ impl StaticJsStrings { (SET, "Set"), (STRING, "String"), (SYMBOL, "Symbol"), + (FINALIZATION_REGISTRY, "FinalizationRegistry"), (TYPED_ARRAY, "TypedArray"), (INT8_ARRAY, "Int8Array"), (UINT8_ARRAY, "Uint8Array"), @@ -337,6 +338,7 @@ const RAW_STATICS: &[StaticString] = &[ StaticString::new(JsStr::latin1("WeakRef".as_bytes())), StaticString::new(JsStr::latin1("WeakMap".as_bytes())), StaticString::new(JsStr::latin1("WeakSet".as_bytes())), + StaticString::new(JsStr::latin1("FinalizationRegistry".as_bytes())), StaticString::new(JsStr::latin1("Temporal".as_bytes())), StaticString::new(JsStr::latin1("Temporal.Now".as_bytes())), StaticString::new(JsStr::latin1("Temporal.Instant".as_bytes())), diff --git a/test262_config.toml b/test262_config.toml index fcc0f5927bb..3d999273a0c 100644 --- a/test262_config.toml +++ b/test262_config.toml @@ -7,7 +7,6 @@ flags = [] features = [ ### Unimplemented features: - "FinalizationRegistry", "symbols-as-weakmap-keys", "Intl.DisplayNames", "Intl.RelativeTimeFormat", From 756631e797aef20daf3eaea2fb28d28f84914191 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Sun, 15 Mar 2026 23:56:57 -0600 Subject: [PATCH 2/6] Add tests --- .../src/builtins/finalization_registry/mod.rs | 13 +++++------- .../builtins/finalization_registry/tests.rs | 21 +++++++++++++++++++ core/engine/src/builtins/mod.rs | 1 + 3 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 core/engine/src/builtins/finalization_registry/tests.rs diff --git a/core/engine/src/builtins/finalization_registry/mod.rs b/core/engine/src/builtins/finalization_registry/mod.rs index b8ceb36aba7..0b147c6dda1 100644 --- a/core/engine/src/builtins/finalization_registry/mod.rs +++ b/core/engine/src/builtins/finalization_registry/mod.rs @@ -23,11 +23,13 @@ use crate::{ use super::{BuiltInConstructor, BuiltInObject, IntrinsicObject, builder::BuiltInBuilder}; +#[cfg(test)] +mod tests; + /// On GG collection, sends a message to a [`FinalizationRegistry`] indicating that it needs to /// be collected. #[derive(Trace)] -#[boa_gc(unsafe_empty_trace)] -struct CleanupSignaler(Cell>>); +struct CleanupSignaler(#[unsafe_ignore_trace] Cell>>); impl Finalize for CleanupSignaler { fn finalize(&self) { @@ -313,12 +315,7 @@ impl FinalizationRegistry { let _key = cell.target.key(); // TODO: it might be better to add a special ref for the value that // also preserves the original key instead. - // SAFETY: the original key is alive per our previous call to `key`, - // so if this returns `Some`, then the value cannot be collected - // until `key` gets dropped. - unsafe { - cell.target.value_ref().and_then(|v| v.0.take()); - } + cell.target.value().and_then(|v| v.0.take()); // ii. Set removed to true. removed = true; diff --git a/core/engine/src/builtins/finalization_registry/tests.rs b/core/engine/src/builtins/finalization_registry/tests.rs new file mode 100644 index 00000000000..0b3d1419252 --- /dev/null +++ b/core/engine/src/builtins/finalization_registry/tests.rs @@ -0,0 +1,21 @@ +use indoc::indoc; + +use crate::{TestAction, run_test_actions}; + +#[test] +fn finalization_registry_simple() { + run_test_actions([ + TestAction::run(indoc! {r#" + let counter = 0; + const registry = new FinalizationRegistry(() => { + counter++; + }); + + registry.register(["foo"]); + "#}), + TestAction::assert_eq("counter", 0), + TestAction::inspect_context(|_| boa_gc::force_collect()), + TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), + TestAction::assert_eq("counter", 1), + ]); +} diff --git a/core/engine/src/builtins/mod.rs b/core/engine/src/builtins/mod.rs index 3f664c69866..f17088d9776 100644 --- a/core/engine/src/builtins/mod.rs +++ b/core/engine/src/builtins/mod.rs @@ -447,6 +447,7 @@ pub(crate) fn set_default_global_bindings(context: &mut Context) -> JsResult<()> global_binding::(context)?; global_binding::(context)?; global_binding::(context)?; + global_binding::(context)?; #[cfg(feature = "annex-b")] { From ccf30215ebb481e0c04d28bf37cc67cca2204c5f Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Mon, 16 Mar 2026 10:31:16 -0600 Subject: [PATCH 3/6] Adjust CLI executor to handle new finalization jobs --- cli/src/executor.rs | 21 ++++- cli/src/main.rs | 7 +- .../builtins/finalization_registry/tests.rs | 90 ++++++++++++++++--- core/engine/src/job.rs | 14 +-- 4 files changed, 108 insertions(+), 24 deletions(-) diff --git a/cli/src/executor.rs b/cli/src/executor.rs index 3ac190397e1..a32eabe65d2 100644 --- a/cli/src/executor.rs +++ b/cli/src/executor.rs @@ -20,6 +20,7 @@ pub(crate) struct Executor { async_jobs: RefCell>, timeout_jobs: RefCell>>, generic_jobs: RefCell>, + finalization_registry_jobs: RefCell>, printer: SharedExternalPrinterLogger, } @@ -31,6 +32,7 @@ impl Executor { async_jobs: RefCell::default(), timeout_jobs: RefCell::default(), generic_jobs: RefCell::default(), + finalization_registry_jobs: RefCell::default(), printer, } } @@ -96,6 +98,9 @@ impl JobExecutor for Executor { .push(job); } Job::GenericJob(job) => self.generic_jobs.borrow_mut().push_back(job), + Job::FinalizationRegistryCleanupJob(job) => { + self.finalization_registry_jobs.borrow_mut().push_back(job); + } job => self.printer.print(format!("unsupported job type {job:?}")), } } @@ -106,12 +111,17 @@ impl JobExecutor for Executor { async fn run_jobs_async(self: Rc, context: &RefCell<&mut Context>) -> JsResult<()> { let mut group = FutureGroup::new(); + let mut fr_group = FutureGroup::new(); loop { for job in mem::take(&mut *self.async_jobs.borrow_mut()) { group.insert(job.call(context)); } + for job in mem::take(&mut *self.finalization_registry_jobs.borrow_mut()) { + fr_group.insert(job.call(context)); + } + if let Some(Err(e)) = future::poll_once(group.next()).await.flatten() { self.printer.print(uncaught_job_error(&e)); } @@ -120,7 +130,16 @@ impl JobExecutor for Executor { // event loop is cancelled almost immediately after the channel with // the reader gets closed. if self.is_empty() && group.is_empty() { - return Ok(()); + // Run finalizers with a lower priority than every other type of + // job. + if let Some(Err(e)) = future::poll_once(fr_group.next()).await.flatten() { + self.printer.print(uncaught_job_error(&e)); + } + + // Finalizers could enqueue new jobs, so we cannot just exit. + if self.is_empty() { + return Ok(()); + } } { diff --git a/cli/src/main.rs b/cli/src/main.rs index 8c8b0ea6a9e..abb0418a764 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -17,7 +17,7 @@ use crate::logger::SharedExternalPrinterLogger; use async_channel::Sender; use boa_engine::JsValue; use boa_engine::error::JsErasedError; -use boa_engine::job::{JobExecutor, NativeAsyncJob}; +use boa_engine::job::NativeAsyncJob; use boa_engine::{ Context, JsError, Source, builtins::promise::PromiseState, @@ -35,9 +35,7 @@ use color_eyre::{ }; use colored::Colorize; use debug::init_boa_debug_object; -use futures_lite::future; use rustyline::{EditMode, Editor, config::Config, error::ReadlineError}; -use std::cell::RefCell; use std::time::{Duration, Instant}; use std::{ fs::OpenOptions, @@ -671,8 +669,7 @@ fn main() -> Result<()> { }); context.enqueue_job(eval_loop.into()); - let result = future::block_on(executor.run_jobs_async(&RefCell::new(context))) - .map_err(|e| e.into_erased(context)); + let result = context.run_jobs().map_err(|e| e.into_erased(context)); handle.join().expect("failed to join thread"); diff --git a/core/engine/src/builtins/finalization_registry/tests.rs b/core/engine/src/builtins/finalization_registry/tests.rs index 0b3d1419252..602bcc53586 100644 --- a/core/engine/src/builtins/finalization_registry/tests.rs +++ b/core/engine/src/builtins/finalization_registry/tests.rs @@ -1,11 +1,13 @@ -use indoc::indoc; +mod miri { -use crate::{TestAction, run_test_actions}; + use indoc::indoc; -#[test] -fn finalization_registry_simple() { - run_test_actions([ - TestAction::run(indoc! {r#" + use crate::{TestAction, run_test_actions}; + + #[test] + fn finalization_registry_simple() { + run_test_actions([ + TestAction::run(indoc! {r#" let counter = 0; const registry = new FinalizationRegistry(() => { counter++; @@ -13,9 +15,75 @@ fn finalization_registry_simple() { registry.register(["foo"]); "#}), - TestAction::assert_eq("counter", 0), - TestAction::inspect_context(|_| boa_gc::force_collect()), - TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), - TestAction::assert_eq("counter", 1), - ]); + TestAction::assert_eq("counter", 0), + TestAction::inspect_context(|_| boa_gc::force_collect()), + TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), + // Callback should run at least once + TestAction::assert_eq("counter", 1), + ]); + } + + #[test] + fn finalization_registry_unregister() { + run_test_actions([ + TestAction::run(indoc! {r#" + let counter = 0; + const registry = new FinalizationRegistry(() => { + counter++; + }); + + { + let array = ["foo"]; + registry.register(array, undefined, array); + registry.unregister(array); + } + + "#}), + TestAction::assert_eq("counter", 0), + TestAction::inspect_context(|_| boa_gc::force_collect()), + TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), + // Callback shouldn't run + TestAction::assert_eq("counter", 0), + ]); + } + + #[test] + fn finalization_registry_held_value_handover() { + run_test_actions([ + TestAction::run(indoc! {r#" + let counter = 0; + const registry = new FinalizationRegistry((value) => { + counter += value.increment; + }); + + registry.register(["foo"], { increment: 5 }); + "#}), + TestAction::assert_eq("counter", 0), + TestAction::inspect_context(|_| boa_gc::force_collect()), + TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), + // Registry should handover the held value as argument + TestAction::assert_eq("counter", 5), + ]); + } + + #[test] + fn finalization_registry_unrelated_unregister_token() { + run_test_actions([ + TestAction::run(indoc! {r#" + let counter = 0; + + const registry = new FinalizationRegistry((value) => { + counter += 1; + }); + + registry.register(["foo"], undefined, {}); + registry.unregister({}); + "#}), + TestAction::assert_eq("counter", 0), + TestAction::inspect_context(|_| boa_gc::force_collect()), + TestAction::inspect_context(|ctx| ctx.run_jobs().unwrap()), + // Object should not have been unregistered if the token is not the correct one + TestAction::assert_eq("counter", 1), + ]); + } } diff --git a/core/engine/src/job.rs b/core/engine/src/job.rs index 46208b03384..416518ea138 100644 --- a/core/engine/src/job.rs +++ b/core/engine/src/job.rs @@ -547,17 +547,17 @@ pub enum Job { /// /// As described on the [spec's section about execution][execution], /// - /// > Because calling HostEnqueueFinalizationRegistryCleanupJob is optional, - /// > registered objects in a FinalizationRegistry do not necessarily hold - /// > that FinalizationRegistry live. Implementations may omit FinalizationRegistry - /// > callbacks for any reason, e.g., if the FinalizationRegistry itself becomes + /// > Because calling `HostEnqueueFinalizationRegistryCleanupJob` is optional, + /// > registered objects in a `FinalizationRegistry` do not necessarily hold + /// > that `FinalizationRegistry` live. Implementations may omit `FinalizationRegistry` + /// > callbacks for any reason, e.g., if the `FinalizationRegistry` itself becomes /// > dead, or if the application is shutting down. /// /// For this reason, it is recommended to exclude `FinalizationRegistry` cleanup - /// jobs from any condition that returns from [`JobExecutor::run_jobs`]. + /// jobs from any condition that exits from [`JobExecutor::run_jobs`]. /// - /// By the same token, it is recommended to execute [`FinalizationRegistryCleanubJob`] - /// separately from all other enqueued [`NativeAsyncJob`]s, prioritizing the + /// By the same token, it is recommended to execute `FinalizationRegistry` cleanup + /// jobs separately from all other enqueued [`NativeAsyncJob`]s, prioritizing the /// execution of all other jobs if possible. /// /// [spec]: https://tc39.es/ecma262/#sec-weakref-host-hooks From bcbd8b787cebe7018506d1758d2b87221bac9f93 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Mon, 16 Mar 2026 10:46:12 -0600 Subject: [PATCH 4/6] Remove free file --- test_weakmap.rs | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 test_weakmap.rs diff --git a/test_weakmap.rs b/test_weakmap.rs deleted file mode 100644 index e78559c210f..00000000000 --- a/test_weakmap.rs +++ /dev/null @@ -1,15 +0,0 @@ -use boa_engine::{Context, Source}; - -fn main() { - let mut context = Context::default(); - let res = context.eval(Source::from_bytes(r#" - const wm = new WeakMap(); - try { - wm.set(42, "value"); - false - } catch (e) { - e instanceof TypeError - } - "#)).unwrap(); - println!("{}", res.as_boolean().unwrap()); -} From 8342ddb0fd771c6f622ab2a59d970ae8be5cf2a7 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Mon, 16 Mar 2026 10:53:44 -0600 Subject: [PATCH 5/6] Deny warnings on husky script --- .husky/pre-push | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.husky/pre-push b/.husky/pre-push index 3da035457f8..7fa476fefcf 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,5 +1,8 @@ #!/bin/sh +target=$(rustc -vV | awk '/^host/ { print $2 }' | tr '[:lower:]' '[:upper:]' | tr '-' '_') +export CARGO_TARGET_${target}_RUSTFLAGS='-D warnings' + if ! command -v cargo-make >/dev/null 2>&1; then echo "cargo-make is not installed. Install it with:" echo " cargo install cargo-make" From 969309b2012a5decf15fb000a346efeb05d36b0c Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Fri, 20 Mar 2026 03:32:42 -0600 Subject: [PATCH 6/6] Inline CanBeHeldWeakly --- .../src/builtins/finalization_registry/mod.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/core/engine/src/builtins/finalization_registry/mod.rs b/core/engine/src/builtins/finalization_registry/mod.rs index 0b147c6dda1..ad41948e4f0 100644 --- a/core/engine/src/builtins/finalization_registry/mod.rs +++ b/core/engine/src/builtins/finalization_registry/mod.rs @@ -218,6 +218,13 @@ impl FinalizationRegistry { let unregister_token = args.get_or_undefined(2); // 3. If CanBeHeldWeakly(target) is false, throw a TypeError exception. + // + // [`CanBeHeldWeakly ( v )`](https://tc39.es/ecma262/#sec-canbeheldweakly) + // + // 1. If v is an Object, return true. + // 2. If v is a Symbol and KeyForSymbol(v) is undefined, return true. + // 3. Return false. + // // TODO: support Symbols let Some(target_obj) = target.as_object() else { return Err(js_error!( @@ -235,6 +242,13 @@ impl FinalizationRegistry { } // 5. If CanBeHeldWeakly(unregisterToken) is false, then + // + // // [`CanBeHeldWeakly ( v )`](https://tc39.es/ecma262/#sec-canbeheldweakly) + // + // 1. If v is an Object, return true. + // 2. If v is a Symbol and KeyForSymbol(v) is undefined, return true. + // 3. Return false. + // // TODO: support Symbols let unregister_token = match unregister_token.variant() { JsVariant::Object(obj) => Some(WeakGc::new(obj.inner())), @@ -286,6 +300,13 @@ impl FinalizationRegistry { })?; // 3. If CanBeHeldWeakly(unregisterToken) is false, throw a TypeError exception.\ + // + // // [`CanBeHeldWeakly ( v )`](https://tc39.es/ecma262/#sec-canbeheldweakly) + // + // 1. If v is an Object, return true. + // 2. If v is a Symbol and KeyForSymbol(v) is undefined, return true. + // 3. Return false. + // // TODO: support Symbols let unregister_token = args.get_or_undefined(0).as_object(); let unregister_token = unregister_token