mark_sweep: run finalizers for erased/live values

Author: Flamki

oscars/src/collectors/mark_sweep/internals/ephemeron.rs

@@ -142,7 +142,7 @@ pub(crate) const fn vtable_of<K: Trace + 'static, V: Trace + 'static>() -> &'sta
             },
             finalize_fn: |this| unsafe {
                 let ephemeron = this.cast::<PoolItem<Ephemeron<K, V>>>().as_ref().value();
-                Finalize::finalize(ephemeron);
+                Trace::run_finalizer(ephemeron);
             },
             _key_type_id: TypeId::of::<K>(),
             _key_size: size_of::<WeakGcBox<K>>(),

oscars/src/collectors/mark_sweep/internals/gc_box.rs

@@ -6,7 +6,7 @@ use crate::collectors::mark_sweep::Finalize;
 use crate::collectors::mark_sweep::internals::gc_header::{GcHeader, HeaderColor};
 use crate::collectors::mark_sweep::{Trace, TraceColor};
 
-use super::{DropFn, TraceFn, VTable, vtable_of};
+use super::{DropFn, FinalizeFn, TraceFn, VTable, vtable_of};
 
 pub struct NonTraceable(());
 
@@ -166,6 +166,10 @@ impl<T: Trace + ?Sized> GcBox<T> {
         self.vtable.drop_fn()
     }
 
+    pub(crate) fn finalize_fn(&self) -> FinalizeFn {
+        self.vtable.finalize_fn()
+    }
+
     pub(crate) fn size(&self) -> usize {
         self.vtable.size()
     }

oscars/src/collectors/mark_sweep/internals/mod.rs

@@ -4,6 +4,6 @@ mod gc_header;
 mod vtable;
 
 pub(crate) use ephemeron::Ephemeron;
-pub(crate) use vtable::{DropFn, TraceFn, VTable, vtable_of};
+pub(crate) use vtable::{DropFn, FinalizeFn, TraceFn, VTable, vtable_of};
 
 pub use self::gc_box::{GcBox, NonTraceable, WeakGcBox};

oscars/src/collectors/mark_sweep/internals/vtable.rs

@@ -27,12 +27,20 @@ pub(crate) const fn vtable_of<T: Trace + 'static>() -> &'static VTable {
             // SAFETY: The caller must ensure the erased pointer is not dropped or deallocated.
             unsafe { core::ptr::drop_in_place(this.as_mut()) };
         }
+
+        // SAFETY: The caller must ensure that the passed erased pointer is `GcBox<Self>`.
+        unsafe fn finalize_fn(this: GcErasedPointer) {
+            // SAFETY: The caller must ensure that the passed erased pointer is `GcBox<Self>`.
+            let value = unsafe { this.cast::<PoolItem<GcBox<Self>>>().as_ref().value() };
+            Trace::run_finalizer(value);
+        }
     }
 
     impl<T: Trace + 'static> HasVTable for T {
         const VTABLE: &'static VTable = &VTable {
             trace_fn: T::trace_fn,
             drop_fn: T::drop_fn,
+            finalize_fn: T::finalize_fn,
             type_id: TypeId::of::<T>(),
             size: size_of::<GcBox<T>>(),
         };
@@ -43,11 +51,13 @@ pub(crate) const fn vtable_of<T: Trace + 'static>() -> &'static VTable {
 
 pub(crate) type TraceFn = unsafe fn(this: GcErasedPointer, color: TraceColor);
 pub(crate) type DropFn = unsafe fn(this: GcErasedPointer);
+pub(crate) type FinalizeFn = unsafe fn(this: GcErasedPointer);
 
 #[derive(Debug)]
 pub(crate) struct VTable {
     trace_fn: TraceFn,
     drop_fn: DropFn,
+    finalize_fn: FinalizeFn,
     type_id: TypeId,
     size: usize,
 }
@@ -61,6 +71,10 @@ impl VTable {
         self.drop_fn
     }
 
+    pub(crate) fn finalize_fn(&self) -> FinalizeFn {
+        self.finalize_fn
+    }
+
     pub(crate) const fn type_id(&self) -> TypeId {
         self.type_id
     }

oscars/src/collectors/mark_sweep/mod.rs

@@ -183,7 +183,9 @@ impl MarkSweepGarbageCollector {
         let ephemerons = core::mem::take(&mut *self.ephemeron_queue.borrow_mut());
         for ephemeron in ephemerons {
             let ephemeron_ref = unsafe { ephemeron.as_ref() };
-            unsafe { ephemeron_ref.value().drop_fn()(ephemeron) };
+            let vtable = ephemeron_ref.value();
+            unsafe { vtable.finalize_fn()(ephemeron) };
+            unsafe { vtable.drop_fn()(ephemeron) };
             self.allocator
                 .borrow_mut()
                 .free_slot(ephemeron.cast::<u8>());
@@ -192,14 +194,18 @@ impl MarkSweepGarbageCollector {
         let roots = core::mem::take(&mut *self.root_queue.borrow_mut());
         for node in roots {
             let node_ref = unsafe { node.as_ref() };
-            unsafe { node_ref.value().drop_fn()(node) };
+            let gc_box = node_ref.value();
+            unsafe { gc_box.finalize_fn()(node) };
+            unsafe { gc_box.drop_fn()(node) };
             self.allocator.borrow_mut().free_slot(node.cast::<u8>());
         }
 
         let pending_e = core::mem::take(&mut *self.pending_ephemeron_queue.borrow_mut());
         for ephemeron in pending_e {
             let ephemeron_ref = unsafe { ephemeron.as_ref() };
-            unsafe { ephemeron_ref.value().drop_fn()(ephemeron) };
+            let vtable = ephemeron_ref.value();
+            unsafe { vtable.finalize_fn()(ephemeron) };
+            unsafe { vtable.drop_fn()(ephemeron) };
             self.allocator
                 .borrow_mut()
                 .free_slot(ephemeron.cast::<u8>());
@@ -208,7 +214,9 @@ impl MarkSweepGarbageCollector {
         let pending_r = core::mem::take(&mut *self.pending_root_queue.borrow_mut());
         for node in pending_r {
             let node_ref = unsafe { node.as_ref() };
-            unsafe { node_ref.value().drop_fn()(node) };
+            let gc_box = node_ref.value();
+            unsafe { gc_box.finalize_fn()(node) };
+            unsafe { gc_box.drop_fn()(node) };
             self.allocator.borrow_mut().free_slot(node.cast::<u8>());
         }
     }
@@ -298,7 +306,7 @@ impl MarkSweepGarbageCollector {
                 // Check if the value is not reachable, i.e. dead.
                 if !gc_box.is_reachable(color) {
                     // Finalize the dead item
-                    gc_box.finalize();
+                    unsafe { gc_box.finalize_fn()(*node) };
                     // Recheck if the value is now rooted again after finalization.
                     if gc_box.is_rooted() {
                         unsafe { gc_box.trace_fn()(*node, color) };

oscars/src/collectors/mark_sweep/tests.rs

@@ -724,3 +724,93 @@ fn collector_drop_runs_ephemeron_value_destructors_for_live_values() {
         "collector drop should run ephemeron value destructor"
     );
 }
+
+#[test]
+fn collector_drop_runs_finalizers_for_live_gc_values() {
+    use core::cell::Cell;
+    use rust_alloc::rc::Rc;
+
+    struct FinalizeSpy {
+        finalized: Rc<Cell<u32>>,
+    }
+
+    impl Finalize for FinalizeSpy {
+        fn finalize(&self) {
+            self.finalized.set(self.finalized.get() + 1);
+        }
+    }
+
+    // SAFETY: `FinalizeSpy` has no traceable children.
+    unsafe impl Trace for FinalizeSpy {
+        crate::empty_trace!();
+    }
+
+    let finalized = Rc::new(Cell::new(0));
+    {
+        let collector = &mut MarkSweepGarbageCollector::default()
+            .with_page_size(128)
+            .with_heap_threshold(256);
+
+        let _gc = Gc::new_in(
+            FinalizeSpy {
+                finalized: Rc::clone(&finalized),
+            },
+            collector,
+        );
+
+        assert_eq!(finalized.get(), 0);
+    }
+
+    assert_eq!(
+        finalized.get(),
+        1,
+        "collector drop should run value finalizer"
+    );
+}
+
+#[test]
+fn collector_drop_runs_ephemeron_value_finalizers_for_live_values() {
+    use core::cell::Cell;
+    use rust_alloc::rc::Rc;
+
+    struct FinalizeSpy {
+        finalized: Rc<Cell<u32>>,
+    }
+
+    impl Finalize for FinalizeSpy {
+        fn finalize(&self) {
+            self.finalized.set(self.finalized.get() + 1);
+        }
+    }
+
+    // SAFETY: `FinalizeSpy` has no traceable children.
+    unsafe impl Trace for FinalizeSpy {
+        crate::empty_trace!();
+    }
+
+    let finalized = Rc::new(Cell::new(0));
+    {
+        let collector = &mut MarkSweepGarbageCollector::default()
+            .with_page_size(128)
+            .with_heap_threshold(256);
+
+        let mut map = WeakMap::new(collector);
+        let key = Gc::new_in(1u64, collector);
+
+        map.insert(
+            &key,
+            FinalizeSpy {
+                finalized: Rc::clone(&finalized),
+            },
+            collector,
+        );
+
+        assert_eq!(finalized.get(), 0);
+    }
+
+    assert_eq!(
+        finalized.get(),
+        1,
+        "collector drop should run ephemeron value finalizer"
+    );
+}