diff --git a/Cargo.lock b/Cargo.lock index 0d3e419..49f2604 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -391,6 +391,7 @@ dependencies = [ "rustc-hash", "thin-vec", "trybuild", + "typeid", ] [[package]] @@ -699,6 +700,12 @@ dependencies = [ "toml", ] +[[package]] +name = "typeid" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c" + [[package]] name = "unicode-ident" version = "1.0.22" diff --git a/oscars/Cargo.toml b/oscars/Cargo.toml index 491bc64..35f9455 100644 --- a/oscars/Cargo.toml +++ b/oscars/Cargo.toml @@ -12,6 +12,7 @@ thin-vec = { version = "0.2", optional = true } icu_locale_core = { version = "2.2.0", default-features = false, optional = true } either = { version = "1.16.0", optional = true } arrayvec = { version = "0.7.6", optional = true } +typeid = "1.0.3" [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 683c6c2..df5f18c 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -10,6 +10,7 @@ use crate::{ use core::fmt; use core::marker::PhantomData; use core::ops::Deref; +use typeid; /// A transient pointer to a GC-managed value. #[derive(Debug)] @@ -89,10 +90,15 @@ impl<'gc, T: Trace + ?Sized + 'gc> Gc<'gc, T> { } } + /// Returns `true` if the inner value is of type `U`. + /// + /// Uses `typeid::of::()`, sound even when `U` carries a branded + /// lifetime because it properly handles branded lifetimes. This avoids the `T: 'static` restriction while still + /// giving us a stable, unique identity guarantee. #[inline] - pub fn is(&self) -> bool { - let actual_type_name = unsafe { (*self.ptr.as_ptr().as_ptr()).0.type_name }; - actual_type_name == core::any::type_name::() + pub fn is(&self) -> bool { + let actual_type_id = unsafe { (*self.ptr.as_ptr().as_ptr()).0.type_id }; + actual_type_id == typeid::of::() } #[inline] @@ -149,7 +155,7 @@ impl<'gc, T: Trace + ?Sized + 'gc> Deref for Gc<'gc, T> { } impl Finalize for Gc<'_, T> {} -unsafe impl Trace for Gc<'_, T> { +unsafe impl<'gc, T: Trace + ?Sized + 'gc> Trace for Gc<'gc, T> { unsafe fn trace(&self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) { tracer.mark(self); } diff --git a/oscars/src/collectors/mark_sweep_branded/gc_box.rs b/oscars/src/collectors/mark_sweep_branded/gc_box.rs index 833af15..8a2a0d2 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc_box.rs @@ -1,5 +1,6 @@ //! The heap header wrapping every GC-managed value. +use core::any::TypeId; use core::cell::Cell; use core::ptr::NonNull; @@ -32,8 +33,12 @@ pub struct GcBox { pub(crate) drop_fn: DropFn, /// Allocation ID used to validate weak pointers. pub(crate) alloc_id: usize, - /// Type name of the underlying value - pub(crate) type_name: &'static str, + /// Unique identifier for the concrete type `T`. + /// + /// Stored as `typeid::of::()`. This safely erases branded lifetimes + /// (eg. `'gc`) without requiring `T: 'static`, giving us a stable + /// unique identity guarantee for sound downcasting. + pub(crate) type_id: TypeId, /// The user value. pub(crate) value: T, } @@ -42,15 +47,17 @@ impl GcBox { pub(crate) const FREED_ALLOC_ID: usize = usize::MAX; } -impl GcBox { - /// Create a [`GcBox`] for `value`, `color` starts as [`GcColor::White`] +impl GcBox { + /// Create a [`GcBox`] for `value`, `color` starts as [`GcColor::White`]. + /// + /// Requires `T: Trace` for the `TypeId`. pub(crate) fn new(value: T, trace_fn: TraceFn, drop_fn: DropFn, alloc_id: usize) -> Self { Self { color: Cell::new(GcColor::White), trace_fn, drop_fn, alloc_id, - type_name: core::any::type_name::(), + type_id: typeid::of::(), value, } } diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 21f757d..3c026ed 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -47,6 +47,12 @@ pub struct Collector { pub(crate) ephemerons: RefCell>, } +impl Default for Collector { + fn default() -> Self { + Self::new() + } +} + impl Collector { pub fn new() -> Self { Self { diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs index e48b16b..34dffac 100644 --- a/oscars/src/collectors/mark_sweep_branded/trace.rs +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -126,7 +126,7 @@ impl<'a> Tracer<'a> { } } -unsafe impl Trace for &T { +unsafe impl Trace for &T { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } @@ -149,7 +149,6 @@ empty_trace![ bool, isize, usize, - str, i8, u8, i16, @@ -180,6 +179,12 @@ empty_trace![ core::sync::atomic::AtomicUsize, ]; +// str is a DST; we cannot allocate it directly. Use String as the Sized proxy. +unsafe impl Trace for str { + #[inline] + unsafe fn trace(&self, _tracer: &mut Tracer) {} +} + unsafe impl Trace for [T; N] { unsafe fn trace(&self, tracer: &mut Tracer) { for v in self.iter() { @@ -188,6 +193,17 @@ unsafe impl Trace for [T; N] { } } +// Slices [T] cannot be allocated directly in the GC. +unsafe impl Trace for [T] { + #[inline] + unsafe fn trace(&self, tracer: &mut Tracer) { + for v in self { + v.trace(tracer); + } + } +} + +// Box where T: ?Sized. Box is always Sized even for DST contents. unsafe impl Trace for Box { unsafe fn trace(&self, tracer: &mut Tracer) { (**self).trace(tracer); @@ -244,20 +260,12 @@ unsafe impl Trace for LinkedList { } } -unsafe impl Trace for PhantomData { +// PhantomData doesn't trace T, so T need not implement Trace. +unsafe impl Trace for PhantomData { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } -unsafe impl Trace for [T] { - #[inline] - unsafe fn trace(&self, tracer: &mut Tracer) { - for v in self { - v.trace(tracer); - } - } -} - unsafe impl Trace for core::any::TypeId { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} @@ -281,10 +289,11 @@ unsafe impl Trace for OnceCell { } } -unsafe impl Trace for Cow<'static, T> +unsafe impl Trace for Cow<'static, T> where T::Owned: Trace, { + // T is already 'static so we can use it directly as the proxy. unsafe fn trace(&self, tracer: &mut Tracer) { if let Cow::Owned(v) = self { v.trace(tracer); @@ -380,17 +389,20 @@ unsafe impl Trace for rust_alloc::rc::Rc { +// Rc/Arc are reference-counted, not GC-traced. They cannot contain live Gc +// pointers (that would create a cycle the GC cannot see). +unsafe impl Trace for rust_alloc::rc::Rc { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } -unsafe impl Trace for rust_alloc::sync::Arc { +unsafe impl Trace for rust_alloc::sync::Arc { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } -unsafe impl Trace for BTreeMap { +// K is not traced (BTreeMap keys are immutable). +unsafe impl Trace for BTreeMap { unsafe fn trace(&self, tracer: &mut Tracer) { for v in self.values() { v.trace(tracer); @@ -398,7 +410,8 @@ unsafe impl Trace for BTreeMap { } } -unsafe impl Trace for BTreeSet { +// BTreeSet keys are never traced. +unsafe impl Trace for BTreeSet { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) { // BTreeSet keys are immutable and cannot contain Gc pointers @@ -470,7 +483,7 @@ unsafe impl Trace for std::time::SystemTime { } #[cfg(feature = "std")] -unsafe impl Trace for std::collections::HashMap { +unsafe impl Trace for std::collections::HashMap { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for (k, v) in self { @@ -481,7 +494,7 @@ unsafe impl Trace for std::collections::HashMap } #[cfg(feature = "std")] -unsafe impl Trace for std::collections::HashSet { +unsafe impl Trace for std::collections::HashSet { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for v in self { diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index 21564d9..9ff658e 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -63,8 +63,7 @@ impl<'id, T: Trace + ?Sized> WeakGc<'id, T> { /// Returns `true` if the referenced value is still alive. pub fn is_upgradable(&self) -> bool { - let is_valid = unsafe { (*self.ptr.as_ptr().as_ptr()).0.alloc_id == self.alloc_id }; - is_valid + unsafe { (*self.ptr.as_ptr().as_ptr()).0.alloc_id == self.alloc_id } } } diff --git a/oscars/src/collectors/null_collector_branded/cell.rs b/oscars/src/collectors/null_collector_branded/cell.rs index ca36252..6040ab6 100644 --- a/oscars/src/collectors/null_collector_branded/cell.rs +++ b/oscars/src/collectors/null_collector_branded/cell.rs @@ -72,10 +72,14 @@ impl DerefMut for GcRefMut<'_, T> { } } -impl<'a, T: Trace + ?Sized> GcRef<'a, T> { - pub fn clone(orig: &GcRef<'a, T>) -> GcRef<'a, T> { - GcRef(Ref::clone(&orig.0)) +impl<'a, T: Trace + ?Sized> Clone for GcRef<'a, T> { + #[inline] + fn clone(&self) -> Self { + GcRef(Ref::clone(&self.0)) } +} + +impl<'a, T: Trace + ?Sized> GcRef<'a, T> { pub fn map(orig: GcRef<'a, T>, f: F) -> GcRef<'a, U> where F: FnOnce(&T) -> &U, @@ -130,6 +134,7 @@ impl<'a, T: Trace + ?Sized> GcRefMut<'a, T> { impl Finalize for GcRefCell {} unsafe impl Trace for GcRefCell { + // GcRefCell<'gc, T> is branded by T's lifetime. Map to the static form. #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { // SAFETY: We only access the inner value for tracing and do not mutate it. diff --git a/oscars/src/collectors/null_collector_branded/gc.rs b/oscars/src/collectors/null_collector_branded/gc.rs index 5e8667c..1d93714 100644 --- a/oscars/src/collectors/null_collector_branded/gc.rs +++ b/oscars/src/collectors/null_collector_branded/gc.rs @@ -80,10 +80,15 @@ impl<'gc, T: Trace + ?Sized + 'gc> Gc<'gc, T> { } } + /// Returns `true` if the inner value is of type `U`. + /// + /// Uses `typeid::of::()`, sound even when `U` carries a branded + /// lifetime because it properly handles branded lifetimes. This avoids the `T: 'static` restriction while still + /// giving us a stable, unique identity guarantee #[inline] - pub fn is(&self) -> bool { - let actual_type_name = unsafe { (*self.ptr.as_ptr().as_ptr()).0.type_name }; - actual_type_name == core::any::type_name::() + pub fn is(&self) -> bool { + let actual_type_id = unsafe { (*self.ptr.as_ptr().as_ptr()).0.type_id }; + actual_type_id == typeid::of::() } #[inline] @@ -141,7 +146,7 @@ impl<'gc, T: Trace + ?Sized + 'gc> Deref for Gc<'gc, T> { impl Finalize for Gc<'_, T> {} -unsafe impl Trace for Gc<'_, T> { +unsafe impl<'gc, T: Trace + ?Sized + 'gc> Trace for Gc<'gc, T> { unsafe fn trace(&self, tracer: &mut crate::collectors::null_collector_branded::trace::Tracer) { tracer.mark(self); } diff --git a/oscars/src/collectors/null_collector_branded/gc_box.rs b/oscars/src/collectors/null_collector_branded/gc_box.rs index 2514b95..933ba1c 100644 --- a/oscars/src/collectors/null_collector_branded/gc_box.rs +++ b/oscars/src/collectors/null_collector_branded/gc_box.rs @@ -1,6 +1,8 @@ +use core::any::TypeId; use core::ptr::NonNull; use crate::alloc::mempool3::PoolAllocator; +use crate::collectors::null_collector_branded::trace::Trace; pub type DropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); @@ -10,18 +12,24 @@ pub type DropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); pub struct GcBox { /// Type erased finalize and free fn pub(crate) drop_fn: DropFn, - /// Type name of the underlying value - pub(crate) type_name: &'static str, + /// Unique identifier for the concrete type `T`. + /// + /// Stored as `typeid::of::()`. This safely erases branded lifetimes + /// (eg. `'gc`) without requiring `T: 'static`, giving us a stable + /// unique identity guarantee for sound downcasting. + pub(crate) type_id: TypeId, /// User value pub(crate) value: T, } -impl GcBox { - /// Create a [`GcBox`] for `value` +impl GcBox { + /// Create a [`GcBox`] for `value`. + /// + /// Requires `T: Trace` for the `TypeId`. pub(crate) fn new(value: T, drop_fn: DropFn) -> Self { Self { drop_fn, - type_name: core::any::type_name::(), + type_id: typeid::of::(), value, } } diff --git a/oscars/src/collectors/null_collector_branded/mutation_ctx.rs b/oscars/src/collectors/null_collector_branded/mutation_ctx.rs index c1bd788..c07209a 100644 --- a/oscars/src/collectors/null_collector_branded/mutation_ctx.rs +++ b/oscars/src/collectors/null_collector_branded/mutation_ctx.rs @@ -23,12 +23,24 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { /// **Note**: This is a temporary workaround to keep `boa_engine` working. /// It breaks the normal safety rules of the collector, and should only be /// used to support older code that relies on `Default` + /// + /// # Safety + /// + /// `Gc<'gc, T>` and `MutationContext` are both `!Send`, so neither can escape + /// the thread that created them. `Collector::drop` only runs at thread exit, + /// after which no `Gc` on this thread can be accessed. The raw pointer reborrow + /// below is therefore sound ,the reference cannot outlive the TLS slot. #[cfg(feature = "std")] pub fn global() -> Self { std::thread_local! { static COLLECTOR: crate::collectors::null_collector_branded::Collector = crate::collectors::null_collector_branded::Collector::new(); } COLLECTOR.with(|c| { + // SAFETY: `Gc` and `MutationContext` are `!Send`, so they cannot escape + // this thread. `COLLECTOR` is a thread-local whose destructor only runs + // at thread exit, after all thread-local values are inaccessible. + // Therefore, `c` remains valid for at least as long as any `MutationContext` + // or `Gc` that could possibly reference it. let ptr = c as *const crate::collectors::null_collector_branded::Collector; Self { collector: unsafe { &*ptr }, diff --git a/oscars/src/collectors/null_collector_branded/trace.rs b/oscars/src/collectors/null_collector_branded/trace.rs index c16c1f5..567020c 100644 --- a/oscars/src/collectors/null_collector_branded/trace.rs +++ b/oscars/src/collectors/null_collector_branded/trace.rs @@ -57,7 +57,7 @@ impl<'a> Tracer<'a> { } } -unsafe impl Trace for &T { +unsafe impl Trace for &T { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } @@ -116,6 +116,7 @@ unsafe impl Trace for [T; N] { } } +// Slices [T] cannot be allocated directly in the GC. unsafe impl Trace for [T] { unsafe fn trace(&self, tracer: &mut Tracer) { for v in self.iter() { @@ -124,6 +125,7 @@ unsafe impl Trace for [T] { } } +// Box where T: ?Sized. Box is always Sized even for DST contents. unsafe impl Trace for Box { unsafe fn trace(&self, tracer: &mut Tracer) { (**self).trace(tracer); @@ -180,7 +182,8 @@ unsafe impl Trace for LinkedList { } } -unsafe impl Trace for PhantomData { +// PhantomData doesn't trace T, so T need not implement Trace. +unsafe impl Trace for PhantomData { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } @@ -201,10 +204,11 @@ unsafe impl Trace for OnceCell { } } -unsafe impl Trace for Cow<'static, T> +unsafe impl Trace for Cow<'static, T> where T::Owned: Trace, { + // T is already 'static so we can use it directly as the proxy. unsafe fn trace(&self, tracer: &mut Tracer) { if let Cow::Owned(v) = self { v.trace(tracer); @@ -257,17 +261,20 @@ unsafe impl Trace for (A, B, C } } -unsafe impl Trace for rust_alloc::rc::Rc { +// Rc/Arc are reference-counted, not GC-traced. They cannot contain live Gc +// pointers (that would create a cycle the GC cannot see). +unsafe impl Trace for rust_alloc::rc::Rc { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } -unsafe impl Trace for rust_alloc::sync::Arc { +unsafe impl Trace for rust_alloc::sync::Arc { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } -unsafe impl Trace for BTreeMap { +// K is not traced (BTreeMap keys are immutable). +unsafe impl Trace for BTreeMap { unsafe fn trace(&self, tracer: &mut Tracer) { for v in self.values() { v.trace(tracer); @@ -275,11 +282,13 @@ unsafe impl Trace for BTreeMap { } } -unsafe impl Trace for BTreeSet { +// BTreeSet keys are never traced. +unsafe impl Trace for BTreeSet { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} } +// str is a DST, so we cannot allocate it directly. Use String as the Sized proxy. unsafe impl Trace for str { #[inline] unsafe fn trace(&self, _tracer: &mut Tracer) {} @@ -316,7 +325,7 @@ unsafe impl Trace for arrayvec::ArrayVec { } } -unsafe impl Trace for hashbrown::hash_map::HashMap { +unsafe impl Trace for hashbrown::hash_map::HashMap { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for (k, v) in self { @@ -327,7 +336,7 @@ unsafe impl Trace for hashbrown::hash_map::HashMap Trace for hashbrown::hash_set::HashSet { +unsafe impl Trace for hashbrown::hash_set::HashSet { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for v in self { @@ -371,7 +380,7 @@ unsafe impl Trace for std::time::SystemTime { } #[cfg(feature = "std")] -unsafe impl Trace for std::collections::HashMap { +unsafe impl Trace for std::collections::HashMap { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for (k, v) in self { @@ -382,7 +391,7 @@ unsafe impl Trace for std::collections::HashMap } #[cfg(feature = "std")] -unsafe impl Trace for std::collections::HashSet { +unsafe impl Trace for std::collections::HashSet { #[inline] unsafe fn trace(&self, tracer: &mut Tracer) { for v in self {