Skip to content

Commit

Permalink
Finalise Gc<T> objects on a separate thread
Browse files Browse the repository at this point in the history
This prevents deadlocks in user code which acquire locks in drop
methods. The new approach to finalisation works as follows:

1. During the entry_point, a finalisation thread, and an mpsc channel
   are initialised.

2. New threads (including the main thread) are given thread local
   storage (TLS) containing a sender, which is used to pass finalisers
   to the finalisation thread. This does not require synchronisation.

3. At the end of the GC pause, Boehm will run finalisers for objects
   which were not marked. This does not run the drop method for the
   object directly. Instead, it sends the drop method to the
   finalisation thread to be run later.

4. The finalisation thread iterates through the message queue, calling
   the finaliser callback for each unreachable object. This finaliser
   callback amounts to a call to `std::ptr::drop_in_place` on the
   object.

I believe this is sound for three reasons:

1. Finaliser Safety Analysis (FSA) does not allow us to use non-Send and
   non-Sync fields inside a drop method, so a drop method run on another
   thread cannot accidentally access memory in non-thread-safe way.

2. Our modification to the BDWGC [1] which adds a memory fence at the
   end of a GC pause means that when a finaliser is run, it will have an
   up-to-date view of the object graph. If a finalisable object is
   mutated after a collection but before it is finalised, then this is
   still fine. This is because that can only happen using Sync-Send-safe
   containers such as a `Mutex` (as this is the only form of interior
   mutability supported by `Gc<T>`.)

3. Before a finaliser is run, Boehm will re-mark the object (and any
   others which reachable from that finaliser) which means that the
   object's memory will not be swept until at least the next GC (or
   potentially even later, if the finalisation thread has not yet run!)

[1]: softdevteam/bdwgc#3
  • Loading branch information
jacob-hughes committed Jan 23, 2024
1 parent 2a214a6 commit a58c2b8
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 36 deletions.
98 changes: 62 additions & 36 deletions library/std/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,18 @@

use core::{
any::Any,
cell::RefCell,
cmp::Ordering,
fmt,
hash::{Hash, Hasher},
marker::{FinalizerSafe, PhantomData, Unsize},
mem::MaybeUninit,
mem::{transmute, MaybeUninit},
ops::{CoerceUnsized, Deref, DispatchFromDyn, Receiver},
ptr::{drop_in_place, null_mut, NonNull},
};

use crate::{sync::mpsc, sync::mpsc::Sender, thread};

pub use alloc::gc::thread_registered;
pub use alloc::gc::GcAllocator;
pub use core::gc::*;
Expand Down Expand Up @@ -258,11 +261,15 @@ impl<T> Gc<T> {
#[cfg(profile_gc)]
FINALIZERS_REGISTERED.fetch_add(1, atomic::Ordering::Relaxed);

unsafe extern "C" fn finalizer<T>(obj: *mut u8, _meta: *mut u8) {
// This function gets called by Boehm when an object becomes unreachable.
unsafe extern "C" fn finalizer<T>(object: *mut u8, _meta: *mut u8) {
unsafe {
drop_in_place(obj as *mut T);
#[cfg(profile_gc)]
FINALIZERS_COMPLETED.fetch_add(1, atomic::Ordering::Relaxed);
let cb = finalizer_callback::<T>;
let fo = FinalizableObj {
callback: transmute(cb as unsafe extern "C" fn(*mut T)),
object,
};
FINALIZER_QUEUE.with(|q| q.borrow().as_ref().unwrap().send(fo).unwrap());
}
}

Expand All @@ -276,37 +283,6 @@ impl<T> Gc<T> {
)
}
}

#[unstable(feature = "gc", issue = "none")]
pub fn unregister_finalizer(&mut self) {
let ptr = self.ptr.as_ptr() as *mut GcBox<T> as *mut u8;
unsafe {
bdwgc::GC_register_finalizer(
ptr,
None,
::core::ptr::null_mut(),
::core::ptr::null_mut(),
::core::ptr::null_mut(),
);
}
}
}

#[cfg(profile_gc)]
#[derive(Debug)]
pub struct FinalizerInfo {
pub registered: u64,
pub completed: u64,
}

#[cfg(profile_gc)]
impl FinalizerInfo {
pub fn finalizer_info() -> FinalizerInfo {
FinalizerInfo {
registered: FINALIZERS_REGISTERED.load(atomic::Ordering::Relaxed),
completed: FINALIZERS_COMPLETED.load(atomic::Ordering::Relaxed),
}
}
}

impl Gc<dyn Any> {
Expand Down Expand Up @@ -663,3 +639,53 @@ impl<T: ?Sized> AsRef<T> for Gc<T> {
&**self
}
}

thread_local! {
pub static FINALIZER_QUEUE: RefCell<Option<FinalizerQueue>> = RefCell::new(None);
}

type FinalizerQueue = Sender<FinalizableObj>;

#[derive(Debug)]
pub struct FinalizableObj {
callback: FinalizerCallback,
object: *mut u8,
}

unsafe impl Send for FinalizableObj {}

pub(crate) fn init_finalization_thread() {
let (sender, receiver) = mpsc::channel();
FINALIZER_QUEUE.with(|q| *q.borrow_mut() = Some(sender));

thread::spawn(move || {
for finalisable in receiver.iter() {
unsafe { (finalisable.callback)(finalisable.object) };
}
});
}

unsafe extern "C" fn finalizer_callback<T>(obj: *mut T) {
unsafe { drop_in_place(obj as *mut T) };
#[cfg(profile_gc)]
FINALIZERS_COMPLETED.fetch_add(1, atomic::Ordering::Relaxed);
}

type FinalizerCallback = unsafe extern "C" fn(data: *mut u8);

#[cfg(profile_gc)]
#[derive(Debug)]
pub struct FinalizerInfo {
pub registered: u64,
pub completed: u64,
}

#[cfg(profile_gc)]
impl FinalizerInfo {
pub fn finalizer_info() -> FinalizerInfo {
FinalizerInfo {
registered: FINALIZERS_REGISTERED.load(atomic::Ordering::Relaxed),
completed: FINALIZERS_COMPLETED.load(atomic::Ordering::Relaxed),
}
}
}
2 changes: 2 additions & 0 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
// info about the stack bounds.
let thread = Thread::new(Some(rtunwrap!(Ok, CString::new("main"))));
thread_info::set(main_guard, thread);

crate::gc::init_finalization_thread();
}
}

Expand Down
6 changes: 6 additions & 0 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ use crate::any::Any;
use crate::cell::UnsafeCell;
use crate::ffi::{CStr, CString};
use crate::fmt;
use crate::gc::FINALIZER_QUEUE;
use crate::io;
use crate::marker::PhantomData;
use crate::mem::{self, forget};
Expand Down Expand Up @@ -512,11 +513,16 @@ impl Builder {
}

let f = MaybeDangling::new(f);

let fin_q_sender = FINALIZER_QUEUE.with(|q| q.borrow().clone());

let main = move || {
if let Some(name) = their_thread.cname() {
imp::Thread::set_name(name);
}

FINALIZER_QUEUE.with(|q| *q.borrow_mut() = fin_q_sender);

crate::io::set_output_capture(output_capture);

// SAFETY: we constructed `f` initialized.
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/runtime/gc/run_finalizers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use std::gc::{Gc, GcAllocator};
use std::sync::atomic::{self, AtomicUsize};
use std::thread;
use std::time;

struct Finalizable(usize);

Expand All @@ -30,6 +32,10 @@ fn foo() {
fn main() {
foo();
GcAllocator::force_gc();

// Wait enough time for the finaliser thread to finish running.

thread::sleep(time::Duration::from_millis(100));
// On some platforms, the last object might not be finalised because it's
// kept alive by a lingering reference.
assert!(FINALIZER_COUNT.load(atomic::Ordering::Relaxed) >= ALLOCATED_COUNT -1);
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/runtime/gc/unchecked_finalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

use std::gc::{Gc, GcAllocator, FinalizeUnchecked};
use std::sync::atomic::{self, AtomicUsize};
use std::thread;
use std::time;

struct UnsafeContainer(usize);

Expand Down Expand Up @@ -33,6 +35,10 @@ fn foo() {
fn main() {
foo();
GcAllocator::force_gc();

// Wait enough time for the finaliser thread to finish running.
thread::sleep(time::Duration::from_millis(100));

// On some platforms, the last object might not be finalised because it's
// kept alive by a lingering reference.
assert!(FINALIZER_COUNT.load(atomic::Ordering::Relaxed) >= ALLOCATED_COUNT -1);
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/runtime/gc/zero_vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

use std::gc::{Gc, GcAllocator};
use std::sync::atomic::{self, AtomicUsize};
use std::thread;
use std::time;

struct Finalizable(usize);

Expand Down Expand Up @@ -41,6 +43,9 @@ fn main() {

GcAllocator::force_gc();

// Wait enough time for the finaliser thread to finish running.
thread::sleep(time::Duration::from_millis(100));

// This tests that finalisation happened indirectly by trying to overwrite references to live GC
// objects in order for Boehm to consider them dead. This is inherently flaky because we might
// miss some references which linger on the stack or in registers. This tends to happen for the
Expand Down

0 comments on commit a58c2b8

Please sign in to comment.