From 01626b6b84b9bfae8f6f8544571a0457bc0acd74 Mon Sep 17 00:00:00 2001 From: Thalia Nero Date: Thu, 30 Jan 2025 10:19:08 -0600 Subject: [PATCH 1/3] Add test for finalizer ressurection. --- gc/src/gc.rs | 6 +++--- gc/tests/finalize.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/gc/src/gc.rs b/gc/src/gc.rs index 3e1f8f9..06c13cd 100644 --- a/gc/src/gc.rs +++ b/gc/src/gc.rs @@ -298,9 +298,9 @@ fn collect_garbage(st: &mut GcState) { unsafe fn sweep(finalized: Vec>, bytes_allocated: &mut usize) { let _guard = DropGuard::new(); for node in finalized.into_iter().rev() { - if node.this.as_ref().header.is_marked() { - continue; - } + // sanity check. If this trips we have violated an unsafe invarant. + // This won't catch all UB, just direct reclamation of roots!! + assert_eq!(node.this.as_ref().header.roots(), 0, "Reclaimed node should not be rooted"); let incoming = node.incoming; let node = Box::from_raw(node.this.as_ptr()); *bytes_allocated -= mem::size_of_val::>(&*node); diff --git a/gc/tests/finalize.rs b/gc/tests/finalize.rs index f717f84..14e712f 100644 --- a/gc/tests/finalize.rs +++ b/gc/tests/finalize.rs @@ -1,4 +1,4 @@ -use gc::{Finalize, Trace}; +use gc::{Finalize, Gc, GcCell, Trace}; use std::cell::Cell; #[derive(PartialEq, Eq, Debug, Clone, Copy)] @@ -46,3 +46,31 @@ fn drop_triggers_finalize() { } FLAGS.with(|f| assert_eq!(f.get(), Flags(1, 1))); } + +#[derive(Trace)] +struct Ressurection { + escape: Gc>>, + value: Gc, +} + +impl Finalize for Ressurection { + fn finalize(&self) { + *self.escape.borrow_mut() = self.value.clone(); + } +} + +// run this with miri to detect UB +// cargo +nightly miri test -p gc --test finalize +#[test] +fn finalizer_can_ressurect() { + let escape = Gc::new(GcCell::new(Gc::new(String::new()))); + let value = Gc::new(GcCell::new(Ressurection { + escape: escape.clone(), + value: Gc::new(String::from("Hello world")), + })); + drop(value); + + gc::force_collect(); + + assert_eq!(&**escape.borrow(), "Hello world"); +} From 67968435aaaddf4649b050a22fed1abe2a601a6e Mon Sep 17 00:00:00 2001 From: Thalia Nero Date: Thu, 30 Jan 2025 12:37:26 -0600 Subject: [PATCH 2/3] Track finalization of nodes. This reserves another bit in roots to store whether the node has been finalized. In a GC cycle, unmarked nodes are finalized at most once, and a second mark phase collects nodes that have not been ressurected. --- gc/src/gc.rs | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/gc/src/gc.rs b/gc/src/gc.rs index 06c13cd..2e1a66f 100644 --- a/gc/src/gc.rs +++ b/gc/src/gc.rs @@ -52,7 +52,8 @@ thread_local!(static GC_STATE: RefCell = RefCell::new(GcState { })); const MARK_MASK: usize = 1 << (usize::BITS - 1); -const ROOTS_MASK: usize = !MARK_MASK; +const FINALIZED_MASK: usize = 1 << (usize::BITS - 2); +const ROOTS_MASK: usize = !(MARK_MASK | FINALIZED_MASK); const ROOTS_MAX: usize = ROOTS_MASK; // max allowed value of roots pub(crate) struct GcBoxHeader { @@ -81,7 +82,7 @@ impl GcBoxHeader { // abort if the count overflows to prevent `mem::forget` loops // that could otherwise lead to erroneous drops if (roots & ROOTS_MASK) < ROOTS_MAX { - self.roots.set(roots + 1); // we checked that this wont affect the high bit + self.roots.set(roots + 1); // we checked that this wont affect the high bits } else { panic!("roots counter overflow"); } @@ -89,7 +90,7 @@ impl GcBoxHeader { #[inline] pub fn dec_roots(&self) { - self.roots.set(self.roots.get() - 1); // no underflow check + self.roots.set(self.roots.get() - 1); // no underflow check, always nonzero number of roots } #[inline] @@ -97,6 +98,16 @@ impl GcBoxHeader { self.roots.get() & MARK_MASK != 0 } + #[inline] + pub fn is_finalized(&self) -> bool { + self.roots.get() & FINALIZED_MASK != 0 + } + + #[inline] + pub fn set_finalized(&self) { + self.roots.set(self.roots.get() | FINALIZED_MASK) + } + #[inline] pub fn mark(&self) { self.roots.set(self.roots.get() | MARK_MASK); @@ -300,7 +311,11 @@ fn collect_garbage(st: &mut GcState) { for node in finalized.into_iter().rev() { // sanity check. If this trips we have violated an unsafe invarant. // This won't catch all UB, just direct reclamation of roots!! - assert_eq!(node.this.as_ref().header.roots(), 0, "Reclaimed node should not be rooted"); + assert_eq!( + node.this.as_ref().header.roots(), + 0, + "Reclaimed node should not be rooted" + ); let incoming = node.incoming; let node = Box::from_raw(node.this.as_ptr()); *bytes_allocated -= mem::size_of_val::>(&*node); @@ -316,10 +331,16 @@ fn collect_garbage(st: &mut GcState) { if unmarked.is_empty() { return; } - for node in &unmarked { - Trace::finalize_glue(&node.this.as_ref().data); + // finalize unmarked nodes + for node in unmarked { + if !node.this.as_ref().header.is_finalized() { + Trace::finalize_glue(&node.this.as_ref().data); + node.this.as_ref().header.set_finalized(); + } } - mark(head); + // rerun mark phase and reclaim unmarked finalized nodes + let mut unmarked = mark(head); + unmarked.retain(|node| node.this.as_ref().header.is_finalized()); sweep(unmarked, &mut st.stats.bytes_allocated); } } From 3c60262589335e993f2a020fc122a0b7abd0ade1 Mon Sep 17 00:00:00 2001 From: Thalia Nero Date: Thu, 30 Jan 2025 13:39:49 -0600 Subject: [PATCH 3/3] Resurrection --- gc/tests/finalize.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gc/tests/finalize.rs b/gc/tests/finalize.rs index 14e712f..c5ac451 100644 --- a/gc/tests/finalize.rs +++ b/gc/tests/finalize.rs @@ -48,12 +48,12 @@ fn drop_triggers_finalize() { } #[derive(Trace)] -struct Ressurection { +struct Resurrection { escape: Gc>>, value: Gc, } -impl Finalize for Ressurection { +impl Finalize for Resurrection { fn finalize(&self) { *self.escape.borrow_mut() = self.value.clone(); } @@ -62,9 +62,9 @@ impl Finalize for Ressurection { // run this with miri to detect UB // cargo +nightly miri test -p gc --test finalize #[test] -fn finalizer_can_ressurect() { +fn finalizer_can_resurrect() { let escape = Gc::new(GcCell::new(Gc::new(String::new()))); - let value = Gc::new(GcCell::new(Ressurection { + let value = Gc::new(GcCell::new(Resurrection { escape: escape.clone(), value: Gc::new(String::from("Hello world")), }));