From ffa854ce3f8c6dcfdb24721513e55a2a9245910d Mon Sep 17 00:00:00 2001 From: raskad <93457935+lupd@users.noreply.github.com> Date: Sat, 11 Feb 2023 04:50:09 +0000 Subject: [PATCH] Implement `WeakSet` (#2586) This Pull Request changes the following: - Implement `WeakSet` buildin object. - Supersedes #2009 Co-authored-by: raskad <32105367+raskad@users.noreply.github.com> --- boa_engine/src/builtins/mod.rs | 4 + boa_engine/src/builtins/weak_set/mod.rs | 253 ++++++++++++++++++++++++ boa_engine/src/context/intrinsics.rs | 13 ++ boa_engine/src/object/mod.rs | 40 +++- boa_gc/src/internals/mod.rs | 3 + boa_gc/src/internals/weak_map_box.rs | 47 +++++ boa_gc/src/lib.rs | 98 ++++++++- boa_gc/src/pointers/mod.rs | 2 + boa_gc/src/pointers/weak.rs | 22 +++ boa_gc/src/pointers/weak_map.rs | 35 ++++ boa_gc/src/test/mod.rs | 1 + boa_gc/src/test/weak_map.rs | 138 +++++++++++++ 12 files changed, 648 insertions(+), 8 deletions(-) create mode 100644 boa_engine/src/builtins/weak_set/mod.rs create mode 100644 boa_gc/src/internals/weak_map_box.rs create mode 100644 boa_gc/src/pointers/weak_map.rs create mode 100644 boa_gc/src/test/weak_map.rs diff --git a/boa_engine/src/builtins/mod.rs b/boa_engine/src/builtins/mod.rs index d31b6d7feb..ba4e1e0431 100644 --- a/boa_engine/src/builtins/mod.rs +++ b/boa_engine/src/builtins/mod.rs @@ -32,6 +32,7 @@ pub mod symbol; pub mod typed_array; pub mod uri; pub mod weak; +pub mod weak_set; #[cfg(feature = "intl")] pub mod intl; @@ -85,6 +86,7 @@ use crate::{ typed_array::TypedArray, uri::{DecodeUri, DecodeUriComponent, EncodeUri, EncodeUriComponent}, weak::WeakRef, + weak_set::WeakSet, }, context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, js_string, @@ -246,6 +248,7 @@ impl Intrinsics { DecodeUri::init(&intrinsics); DecodeUriComponent::init(&intrinsics); WeakRef::init(&intrinsics); + WeakSet::init(&intrinsics); #[cfg(feature = "intl")] { intl::Intl::init(&intrinsics); @@ -340,6 +343,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 = "intl")] global_binding::(context)?; diff --git a/boa_engine/src/builtins/weak_set/mod.rs b/boa_engine/src/builtins/weak_set/mod.rs new file mode 100644 index 0000000000..e425e60857 --- /dev/null +++ b/boa_engine/src/builtins/weak_set/mod.rs @@ -0,0 +1,253 @@ +//! Boa's implementation of ECMAScript's `WeakSet` builtin object. +//! +//! More information: +//! - [ECMAScript reference][spec] +//! - [MDN documentation][mdn] +//! +//! [spec]: https://tc39.es/ecma262/#sec-weakset-objects +//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet + +use crate::{ + builtins::{BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject}, + context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, + object::{internal_methods::get_prototype_from_constructor, JsObject, ObjectData}, + property::Attribute, + symbol::JsSymbol, + Context, JsArgs, JsNativeError, JsResult, JsValue, +}; +use boa_gc::{Finalize, Trace, WeakMap}; +use boa_profiler::Profiler; + +#[derive(Debug, Trace, Finalize)] +pub(crate) struct WeakSet; + +impl IntrinsicObject for WeakSet { + fn get(intrinsics: &Intrinsics) -> JsObject { + Self::STANDARD_CONSTRUCTOR(intrinsics.constructors()).constructor() + } + + fn init(intrinsics: &Intrinsics) { + let _timer = Profiler::global().start_event(Self::NAME, "init"); + BuiltInBuilder::from_standard_constructor::(intrinsics) + .property( + JsSymbol::to_string_tag(), + Self::NAME, + Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + ) + .method(Self::add, "add", 1) + .method(Self::delete, "delete", 1) + .method(Self::has, "has", 1) + .build(); + } +} + +impl BuiltInObject for WeakSet { + const NAME: &'static str = "WeakSet"; + + const ATTRIBUTE: Attribute = Attribute::WRITABLE.union(Attribute::CONFIGURABLE); +} + +impl BuiltInConstructor for WeakSet { + /// The amount of arguments the `WeakSet` constructor takes. + const LENGTH: usize = 0; + + const STANDARD_CONSTRUCTOR: fn(&StandardConstructors) -> &StandardConstructor = + StandardConstructors::weak_set; + + /// `WeakSet ( [ iterable ] )` + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakset-iterable + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet/WeakSet + 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(JsNativeError::typ() + .with_message("WeakSet: cannot call constructor without `new`") + .into()); + } + + // 2. Let set be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakSet.prototype%", « [[WeakSetData]] »). + // 3. Set set.[[WeakSetData]] to a new empty List. + let weak_set = JsObject::from_proto_and_data( + get_prototype_from_constructor(new_target, StandardConstructors::weak_set, context)?, + ObjectData::weak_set(WeakMap::new()), + ); + + // 4. If iterable is either undefined or null, return set. + let iterable = args.get_or_undefined(0); + if iterable.is_null_or_undefined() { + return Ok(weak_set.into()); + } + + // 5. Let adder be ? Get(set, "add"). + let adder = weak_set.get("add", context)?; + + // 6. If IsCallable(adder) is false, throw a TypeError exception. + let adder = adder + .as_callable() + .ok_or_else(|| JsNativeError::typ().with_message("WeakSet: 'add' is not a function"))?; + + // 7. Let iteratorRecord be ? GetIterator(iterable). + let iterator_record = iterable.clone().get_iterator(context, None, None)?; + + // 8. Repeat, + // a. Let next be ? IteratorStep(iteratorRecord). + while let Some(next) = iterator_record.step(context)? { + // c. Let nextValue be ? IteratorValue(next). + let next_value = next.value(context)?; + + // d. Let status be Completion(Call(adder, set, « nextValue »)). + // e. IfAbruptCloseIterator(status, iteratorRecord). + if let Err(status) = adder.call(&weak_set.clone().into(), &[next_value], context) { + return iterator_record.close(Err(status), context); + } + } + + // b. If next is false, return set. + Ok(weak_set.into()) + } +} + +impl WeakSet { + /// `WeakSet.prototype.add( value )` + /// + /// The add() method appends a new object to the end of a `WeakSet` object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakset.prototype.add + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet/add + pub(crate) fn add( + this: &JsValue, + args: &[JsValue], + _context: &mut Context<'_>, + ) -> JsResult { + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[WeakSetData]]). + let Some(obj) = this.as_object() else { + return Err(JsNativeError::typ() + .with_message("WeakSet.add: called with non-object value") + .into()); + }; + let mut obj_borrow = obj.borrow_mut(); + let o = obj_borrow.as_weak_set_mut().ok_or_else(|| { + JsNativeError::typ().with_message("WeakSet.add: called with non-object value") + })?; + + // 3. If Type(value) is not Object, throw a TypeError exception. + let value = args.get_or_undefined(0); + let Some(value) = args.get_or_undefined(0).as_object() else { + return Err(JsNativeError::typ() + .with_message(format!( + "WeakSet.add: expected target argument of type `object`, got target of type `{}`", + value.type_of() + )).into()); + }; + + // 4. Let entries be the List that is S.[[WeakSetData]]. + // 5. For each element e of entries, do + if o.contains_key(value.inner()) { + // a. If e is not empty and SameValue(e, value) is true, then + // i. Return S. + return Ok(this.clone()); + } + + // 6. Append value as the last element of entries. + o.insert(value.inner(), ()); + + // 7. Return S. + Ok(this.clone()) + } + + /// `WeakSet.prototype.delete( value )` + /// + /// The delete() method removes the specified element from a `WeakSet` object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakset.prototype.delete + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet/delete + pub(crate) fn delete( + this: &JsValue, + args: &[JsValue], + _context: &mut Context<'_>, + ) -> JsResult { + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[WeakSetData]]). + let Some(obj) = this.as_object() else { + return Err(JsNativeError::typ() + .with_message("WeakSet.delete: called with non-object value") + .into()); + }; + let mut obj_borrow = obj.borrow_mut(); + let o = obj_borrow.as_weak_set_mut().ok_or_else(|| { + JsNativeError::typ().with_message("WeakSet.delete: called with non-object value") + })?; + + // 3. If Type(value) is not Object, return false. + let value = args.get_or_undefined(0); + let Some(value) = value.as_object() else { + return Ok(false.into()); + }; + + // 4. Let entries be the List that is S.[[WeakSetData]]. + // 5. For each element e of entries, do + // a. If e is not empty and SameValue(e, value) is true, then + // i. Replace the element of entries whose value is e with an element whose value is empty. + // ii. Return true. + // 6. Return false. + Ok(o.remove(value.inner()).is_some().into()) + } + + /// `WeakSet.prototype.has( value )` + /// + /// The has() method returns a boolean indicating whether an object exists in a `WeakSet` or not. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakset.prototype.has + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet/has + pub(crate) fn has( + this: &JsValue, + args: &[JsValue], + _context: &mut Context<'_>, + ) -> JsResult { + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[WeakSetData]]). + let Some(obj) = this.as_object() else { + return Err(JsNativeError::typ() + .with_message("WeakSet.has: called with non-object value") + .into()); + }; + let obj_borrow = obj.borrow(); + let o = obj_borrow.as_weak_set().ok_or_else(|| { + JsNativeError::typ().with_message("WeakSet.has: called with non-object value") + })?; + + // 3. Let entries be the List that is S.[[WeakSetData]]. + // 4. If Type(value) is not Object, return false. + let value = args.get_or_undefined(0); + let Some(value) = value.as_object() else { + return Ok(false.into()); + }; + + // 5. For each element e of entries, do + // a. If e is not empty and SameValue(e, value) is true, return true. + // 6. Return false. + Ok(o.contains_key(value.inner()).into()) + } +} diff --git a/boa_engine/src/context/intrinsics.rs b/boa_engine/src/context/intrinsics.rs index 392fc1f103..8717a13a25 100644 --- a/boa_engine/src/context/intrinsics.rs +++ b/boa_engine/src/context/intrinsics.rs @@ -114,6 +114,7 @@ pub struct StandardConstructors { date_time_format: StandardConstructor, promise: StandardConstructor, weak_ref: StandardConstructor, + weak_set: StandardConstructor, #[cfg(feature = "intl")] collator: StandardConstructor, #[cfg(feature = "intl")] @@ -180,6 +181,7 @@ impl Default for StandardConstructors { date_time_format: StandardConstructor::default(), promise: StandardConstructor::default(), weak_ref: StandardConstructor::default(), + weak_set: StandardConstructor::default(), #[cfg(feature = "intl")] collator: StandardConstructor::default(), #[cfg(feature = "intl")] @@ -644,6 +646,17 @@ impl StandardConstructors { &self.weak_ref } + /// Returns the `WeakSet` constructor. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-weakset-constructor + #[inline] + pub const fn weak_set(&self) -> &StandardConstructor { + &self.weak_set + } + /// Returns the `Intl.Collator` constructor. /// /// More information: diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 587df48e48..0793a7527b 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -54,7 +54,7 @@ use crate::{ Context, JsBigInt, JsString, JsSymbol, JsValue, }; -use boa_gc::{custom_trace, Finalize, GcRefCell, Trace, WeakGc}; +use boa_gc::{custom_trace, Finalize, GcRefCell, Trace, WeakGc, WeakMap}; use std::{ any::Any, fmt::{self, Debug}, @@ -270,6 +270,9 @@ pub enum ObjectKind { /// The `WeakRef` object kind. WeakRef(WeakGc>), + /// The `WeakSet` object kind. + WeakSet(WeakMap, ()>), + /// The `Intl.Collator` object kind. #[cfg(feature = "intl")] Collator(Box), @@ -311,6 +314,7 @@ unsafe impl Trace for ObjectKind { Self::Promise(p) => mark(p), Self::AsyncGenerator(g) => mark(g), Self::WeakRef(wr) => mark(wr), + Self::WeakSet(ws) => mark(ws), #[cfg(feature = "intl")] Self::DateTimeFormat(f) => mark(f), #[cfg(feature = "intl")] @@ -627,6 +631,15 @@ impl ObjectData { } } + /// Create the `WeakSet` object data + #[must_use] + pub fn weak_set(weak_set: WeakMap, ()>) -> Self { + Self { + kind: ObjectKind::WeakSet(weak_set), + internal_methods: &ORDINARY_INTERNAL_METHODS, + } + } + /// Create the `NativeObject` object data #[must_use] pub fn native_object(native_object: T) -> Self { @@ -722,6 +735,7 @@ impl Debug for ObjectKind { Self::DataView(_) => "DataView", Self::Promise(_) => "Promise", Self::WeakRef(_) => "WeakRef", + Self::WeakSet(_) => "WeakSet", #[cfg(feature = "intl")] Self::Collator(_) => "Collator", #[cfg(feature = "intl")] @@ -1539,6 +1553,30 @@ impl Object { } } + /// Gets the weak set data if the object is a `WeakSet`. + #[inline] + pub const fn as_weak_set(&self) -> Option<&WeakMap, ()>> { + match self.data { + ObjectData { + kind: ObjectKind::WeakSet(ref weak_set), + .. + } => Some(weak_set), + _ => None, + } + } + + /// Gets the mutable weak set data if the object is a `WeakSet`. + #[inline] + pub fn as_weak_set_mut(&mut self) -> Option<&mut WeakMap, ()>> { + match self.data { + ObjectData { + kind: ObjectKind::WeakSet(ref mut weak_set), + .. + } => Some(weak_set), + _ => None, + } + } + /// Gets the prototype instance of this object. #[inline] pub const fn prototype(&self) -> &JsPrototype { diff --git a/boa_gc/src/internals/mod.rs b/boa_gc/src/internals/mod.rs index f308f763da..964b526e84 100644 --- a/boa_gc/src/internals/mod.rs +++ b/boa_gc/src/internals/mod.rs @@ -1,5 +1,8 @@ mod ephemeron_box; mod gc_box; +mod weak_map_box; pub(crate) use self::ephemeron_box::{EphemeronBox, ErasedEphemeronBox}; +pub(crate) use self::weak_map_box::{ErasedWeakMapBox, WeakMapBox}; + pub use self::gc_box::GcBox; diff --git a/boa_gc/src/internals/weak_map_box.rs b/boa_gc/src/internals/weak_map_box.rs new file mode 100644 index 0000000000..bd1a355baf --- /dev/null +++ b/boa_gc/src/internals/weak_map_box.rs @@ -0,0 +1,47 @@ +use crate::{GcRefCell, Trace, WeakGc}; +use std::{cell::Cell, collections::HashMap, ptr::NonNull}; + +/// A box that is used to track [`WeakMap`][`crate::WeakMap`]s. +pub(crate) struct WeakMapBox { + pub(crate) map: WeakGc, V>>>, + pub(crate) next: Cell>>, +} + +/// A trait that is used to erase the type of a [`WeakMapBox`]. +pub(crate) trait ErasedWeakMapBox { + /// Clear dead entries from the [`WeakMapBox`]. + fn clear_dead_entries(&self); + + /// A pointer to the next [`WeakMapBox`]. + fn next(&self) -> &Cell>>; + + /// Returns `true` if the [`WeakMapBox`] is live. + fn is_live(&self) -> bool; + + /// Traces the weak reference inside of the [`WeakMapBox`] it the weak map is live. + unsafe fn trace(&self); +} + +impl ErasedWeakMapBox for WeakMapBox { + fn clear_dead_entries(&self) { + if let Some(map) = self.map.upgrade() { + let mut map = map.borrow_mut(); + map.retain(|k, _| k.upgrade().is_some()); + } + } + + fn next(&self) -> &Cell>> { + &self.next + } + + fn is_live(&self) -> bool { + self.map.upgrade().is_some() + } + + unsafe fn trace(&self) { + if self.map.upgrade().is_some() { + // SAFETY: When the weak map is live, the weak reference should be traced. + unsafe { self.map.trace() } + } + } +} diff --git a/boa_gc/src/lib.rs b/boa_gc/src/lib.rs index 49158b1936..dc207ffe02 100644 --- a/boa_gc/src/lib.rs +++ b/boa_gc/src/lib.rs @@ -96,9 +96,10 @@ mod trace; pub(crate) mod internals; use boa_profiler::Profiler; -use internals::{EphemeronBox, ErasedEphemeronBox}; +use internals::{EphemeronBox, ErasedEphemeronBox, ErasedWeakMapBox, WeakMapBox}; use std::{ cell::{Cell, RefCell}, + collections::HashMap, mem, ptr::NonNull, }; @@ -107,17 +108,19 @@ pub use crate::trace::{Finalize, Trace}; pub use boa_macros::{Finalize, Trace}; pub use cell::{GcRef, GcRefCell, GcRefMut}; pub use internals::GcBox; -pub use pointers::{Ephemeron, Gc, WeakGc}; +pub use pointers::{Ephemeron, Gc, WeakGc, WeakMap}; type GcPointer = NonNull>; type EphemeronPointer = NonNull; +type ErasedWeakMapBoxPointer = NonNull; thread_local!(static GC_DROPPING: Cell = Cell::new(false)); thread_local!(static BOA_GC: RefCell = RefCell::new( BoaGc { config: GcConfig::default(), runtime: GcRuntimeData::default(), strong_start: Cell::new(None), - weak_start: Cell::new(None) + weak_start: Cell::new(None), + weak_map_start: Cell::new(None), })); #[derive(Debug, Clone, Copy)] @@ -150,6 +153,7 @@ struct BoaGc { runtime: GcRuntimeData, strong_start: Cell>, weak_start: Cell>, + weak_map_start: Cell>, } impl Drop for BoaGc { @@ -234,6 +238,32 @@ impl Allocator { }) } + fn alloc_weak_map() -> WeakMap { + let _timer = Profiler::global().start_event("New WeakMap", "BoaAlloc"); + + let weak_map = WeakMap { + inner: Gc::new(GcRefCell::new(HashMap::new())), + }; + let weak = WeakGc::new(&weak_map.inner); + + BOA_GC.with(|st| { + let gc = st.borrow_mut(); + + let weak_box = WeakMapBox { + map: weak, + next: Cell::new(gc.weak_map_start.take()), + }; + + // Safety: value cannot be a null pointer, since `Box` cannot return null pointers. + let ptr = unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(weak_box))) }; + let erased: ErasedWeakMapBoxPointer = ptr; + + gc.weak_map_start.set(Some(erased)); + + weak_map + }) + } + fn manage_state(gc: &mut BoaGc) { if gc.runtime.bytes_allocated > gc.config.threshold { Collector::collect(gc); @@ -272,7 +302,7 @@ impl Collector { fn collect(gc: &mut BoaGc) { let _timer = Profiler::global().start_event("Gc Full Collection", "gc"); gc.runtime.collections += 1; - let unreachables = Self::mark_heap(&gc.strong_start, &gc.weak_start); + let unreachables = Self::mark_heap(&gc.strong_start, &gc.weak_start, &gc.weak_map_start); // Only finalize if there are any unreachable nodes. if !unreachables.strong.is_empty() || unreachables.weak.is_empty() { @@ -280,7 +310,8 @@ impl Collector { // SAFETY: All passed pointers are valid, since we won't deallocate until `Self::sweep`. unsafe { Self::finalize(unreachables) }; - let _final_unreachables = Self::mark_heap(&gc.strong_start, &gc.weak_start); + let _final_unreachables = + Self::mark_heap(&gc.strong_start, &gc.weak_start, &gc.weak_map_start); } // SAFETY: The head of our linked list is always valid per the invariants of our GC. @@ -291,12 +322,32 @@ impl Collector { &mut gc.runtime.bytes_allocated, ); } + + // Weak maps have to be cleared after the sweep, since the process dereferences GcBoxes. + let mut weak_map = &gc.weak_map_start; + while let Some(w) = weak_map.get() { + // SAFETY: The caller must ensure the validity of every node of `heap_start`. + let node_ref = unsafe { w.as_ref() }; + + if node_ref.is_live() { + node_ref.clear_dead_entries(); + weak_map = node_ref.next(); + } else { + weak_map.set(node_ref.next().take()); + + // SAFETY: + // The `Allocator` must always ensure its start node is a valid, non-null pointer that + // was allocated by `Box::from_raw(Box::new(..))`. + let _unmarked_node = unsafe { Box::from_raw(w.as_ptr()) }; + } + } } /// Walk the heap and mark any nodes deemed reachable fn mark_heap( mut strong: &Cell>>>, mut weak: &Cell>>, + mut weak_map: &Cell>, ) -> Unreachables { let _timer = Profiler::global().start_event("Gc Marking", "gc"); // Walk the list, tracing and marking the nodes @@ -348,7 +399,18 @@ impl Collector { weak = &eph_ref.header().next; } - // 2. Iterate through all pending ephemerons, removing the ones which have been successfully + // 2. Trace all the weak pointers in the live weak maps to make sure they do not get swept. + while let Some(w) = weak_map.get() { + // SAFETY: node must be valid as this phase cannot drop any node. + let node_ref = unsafe { w.as_ref() }; + + // SAFETY: The garbage collector ensures that all nodes are valid. + unsafe { node_ref.trace() }; + + weak_map = node_ref.next(); + } + + // 3. Iterate through all pending ephemerons, removing the ones which have been successfully // traced. If there are no changes in the pending ephemerons list, it means that there are no // more reachable ephemerons from the remaining ephemeron values. let mut previous_len = pending_ephemerons.len(); @@ -367,7 +429,7 @@ impl Collector { previous_len = pending_ephemerons.len(); } - // 3. The remaining list should contain the ephemerons that are either unreachable or its key + // 4. The remaining list should contain the ephemerons that are either unreachable or its key // is dead. Cleanup the strong pointers since this procedure could have marked some more strong // pointers. strong_dead.retain_mut(|node| { @@ -448,6 +510,17 @@ impl Collector { // Clean up the heap when BoaGc is dropped fn dump(gc: &mut BoaGc) { + // Weak maps have to be dropped first, since the process dereferences GcBoxes. + // This can be done without initializing a dropguard since no GcBox's are being dropped. + let weak_map_head = &gc.weak_map_start; + while let Some(node) = weak_map_head.get() { + // SAFETY: + // The `Allocator` must always ensure its start node is a valid, non-null pointer that + // was allocated by `Box::from_raw(Box::new(..))`. + let unmarked_node = unsafe { Box::from_raw(node.as_ptr()) }; + weak_map_head.set(unmarked_node.next().take()); + } + // Not initializing a dropguard since this should only be invoked when BOA_GC is being dropped. let _guard = DropGuard::new(); @@ -484,3 +557,14 @@ pub fn force_collect() { #[cfg(test)] mod test; + +/// Returns `true` is any weak maps are currently allocated. +#[cfg(test)] +#[must_use] +pub fn has_weak_maps() -> bool { + BOA_GC.with(|current| { + let gc = current.borrow(); + + gc.weak_map_start.get().is_some() + }) +} diff --git a/boa_gc/src/pointers/mod.rs b/boa_gc/src/pointers/mod.rs index aeb5495373..fee764b3af 100644 --- a/boa_gc/src/pointers/mod.rs +++ b/boa_gc/src/pointers/mod.rs @@ -4,7 +4,9 @@ mod ephemeron; mod gc; mod rootable; mod weak; +mod weak_map; pub use ephemeron::Ephemeron; pub use gc::Gc; pub use weak::WeakGc; +pub use weak_map::WeakMap; diff --git a/boa_gc/src/pointers/weak.rs b/boa_gc/src/pointers/weak.rs index 3b6bfc366f..edeadd7add 100644 --- a/boa_gc/src/pointers/weak.rs +++ b/boa_gc/src/pointers/weak.rs @@ -1,4 +1,5 @@ use crate::{Ephemeron, Finalize, Gc, Trace}; +use std::hash::{Hash, Hasher}; /// A weak reference to a [`Gc`]. /// @@ -37,3 +38,24 @@ impl From>> for WeakGc { Self { inner } } } + +impl PartialEq for WeakGc { + fn eq(&self, other: &Self) -> bool { + match (self.upgrade(), other.upgrade()) { + (Some(a), Some(b)) => std::ptr::eq(a.as_ref(), b.as_ref()), + _ => false, + } + } +} + +impl Eq for WeakGc {} + +impl Hash for WeakGc { + fn hash(&self, state: &mut H) { + if let Some(obj) = self.upgrade() { + std::ptr::hash(obj.as_ref(), state); + } else { + std::ptr::hash(self, state); + } + } +} diff --git a/boa_gc/src/pointers/weak_map.rs b/boa_gc/src/pointers/weak_map.rs new file mode 100644 index 0000000000..1006e1cea3 --- /dev/null +++ b/boa_gc/src/pointers/weak_map.rs @@ -0,0 +1,35 @@ +use crate::{Allocator, Finalize, Gc, GcRefCell, Trace, WeakGc}; +use std::collections::HashMap; + +/// A map that holds weak references to its keys and is traced by the garbage collector. +#[derive(Clone, Debug, Default, Trace, Finalize)] +pub struct WeakMap { + pub(crate) inner: Gc, V>>>, +} + +impl WeakMap { + /// Creates a new [`WeakMap`]. + #[must_use] + #[inline] + pub fn new() -> Self { + Allocator::alloc_weak_map() + } + + /// Inserts a key-value pair into the map. + #[inline] + pub fn insert(&mut self, key: &Gc, value: V) { + self.inner.borrow_mut().insert(WeakGc::new(key), value); + } + + /// Removes a key from the map, returning the value at the key if the key was previously in the map. + #[inline] + pub fn remove(&mut self, key: &Gc) -> Option { + self.inner.borrow_mut().remove(&WeakGc::new(key)) + } + + /// Returns `true` if the map contains a value for the specified key. + #[inline] + pub fn contains_key(&self, key: &Gc) -> bool { + self.inner.borrow().contains_key(&WeakGc::new(key)) + } +} diff --git a/boa_gc/src/test/mod.rs b/boa_gc/src/test/mod.rs index 81aadb27f3..fbe6f7939b 100644 --- a/boa_gc/src/test/mod.rs +++ b/boa_gc/src/test/mod.rs @@ -3,6 +3,7 @@ use crate::BOA_GC; mod allocation; mod cell; mod weak; +mod weak_map; struct Harness; diff --git a/boa_gc/src/test/weak_map.rs b/boa_gc/src/test/weak_map.rs new file mode 100644 index 0000000000..36c3488190 --- /dev/null +++ b/boa_gc/src/test/weak_map.rs @@ -0,0 +1,138 @@ +use super::run_test; +use crate::{force_collect, has_weak_maps, Gc, WeakMap}; + +#[test] +fn weak_map_basic() { + run_test(|| { + let key1 = Gc::new(String::from("key1")); + let key2 = Gc::new(String::from("key2")); + let key3 = Gc::new(String::from("key3")); + + assert!(!has_weak_maps()); + + let mut map = WeakMap::new(); + + assert!(has_weak_maps()); + + map.insert(&key1, ()); + map.insert(&key2, ()); + map.insert(&key3, ()); + + force_collect(); + assert!(has_weak_maps()); + + assert!(map.contains_key(&key1)); + assert!(map.contains_key(&key2)); + assert!(map.contains_key(&key3)); + + drop(key1); + + force_collect(); + assert!(has_weak_maps()); + + assert!(map.contains_key(&key2)); + assert!(map.contains_key(&key3)); + + drop(key2); + + force_collect(); + assert!(has_weak_maps()); + + assert!(map.contains_key(&key3)); + assert!(has_weak_maps()); + + drop(key3); + + assert!(has_weak_maps()); + + force_collect(); + assert!(has_weak_maps()); + + drop(map); + + force_collect(); + assert!(!has_weak_maps()); + }); +} + +#[test] +fn weak_map_multiple() { + run_test(|| { + let key1 = Gc::new(String::from("key1")); + let key2 = Gc::new(String::from("key2")); + let key3 = Gc::new(String::from("key3")); + + assert!(!has_weak_maps()); + + let mut map_1 = WeakMap::new(); + let mut map_2 = WeakMap::new(); + + assert!(has_weak_maps()); + + map_1.insert(&key1, ()); + map_1.insert(&key2, ()); + map_2.insert(&key3, ()); + + force_collect(); + assert!(has_weak_maps()); + + assert!(map_1.contains_key(&key1)); + assert!(map_1.contains_key(&key2)); + assert!(!map_1.contains_key(&key3)); + assert!(!map_2.contains_key(&key1)); + assert!(!map_2.contains_key(&key2)); + assert!(map_2.contains_key(&key3)); + + force_collect(); + assert!(has_weak_maps()); + + drop(key1); + drop(key2); + + force_collect(); + assert!(has_weak_maps()); + + assert!(!map_1.contains_key(&key3)); + assert!(map_2.contains_key(&key3)); + + drop(key3); + + force_collect(); + assert!(has_weak_maps()); + + drop(map_1); + + force_collect(); + assert!(has_weak_maps()); + + drop(map_2); + + force_collect(); + assert!(!has_weak_maps()); + }); +} + +#[test] +fn weak_map_key_live() { + run_test(|| { + let key = Gc::new(String::from("key")); + let key_copy = key.clone(); + + let mut map = WeakMap::new(); + + map.insert(&key, ()); + + assert!(map.contains_key(&key)); + assert!(map.contains_key(&key_copy)); + + assert_eq!(map.remove(&key), Some(())); + + map.insert(&key, ()); + + drop(key); + + force_collect(); + + assert!(map.contains_key(&key_copy)); + }); +}