Skip to content

Commit

Permalink
Merge pull request #234 from jacob-hughes/remove_finalizer_safe
Browse files Browse the repository at this point in the history
Remove FinalizerSafe trait
  • Loading branch information
ltratt authored Dec 2, 2024
2 parents ced805b + c121c88 commit c0a71d6
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 24 deletions.
8 changes: 0 additions & 8 deletions src/lib/vm/objects/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,6 @@ pub struct Block {
pub method_stack_base: usize,
}

// The following impls are needed to allow the GC to finalise `Block`. By
// default, `Block` is not `FinalizerSafe` because it has fields to other `Gc`s.
// These fields would be unsound to access inside a `drop` method, so this
// explicit impl is needed to tell the compiler we are not doing that. The only
// time a `Block` is shared between threads is inside a finaliser, which is
// Sync-safe because we guarantee it will not be accessed at the same time as
// the main-thread.
unsafe impl FinalizerSafe for Block {}
unsafe impl Sync for Block {}

impl Obj for Block {
Expand Down
5 changes: 0 additions & 5 deletions src/lib/vm/objects/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ pub struct Class {
inst_vars: UnsafeCell<Vec<Val>>,
}

// This is safe because there is no non-GC'd shared ownership inside `Class`.
// This means that even though a finalizer will call its drop methods in another
// thread, it is guaranteed that the finalizer thread will be the only thread
// accessing its data.
unsafe impl FinalizerSafe for Class {}
unsafe impl Sync for Class {}

impl Obj for Class {
Expand Down
5 changes: 0 additions & 5 deletions src/lib/vm/objects/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ pub struct Method {
pub func: Function,
}

// By default, `Method` is not `FinalizerSafe` because it contains a `Cell`
// field. Cell is `!Sync` so it would be unsound to access inside a `drop`
// method. This explicit impl is needed to tell the compiler we are not doing
// that.
unsafe impl FinalizerSafe for Method {}
unsafe impl Sync for Method {}

impl Obj for Method {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/vm/objects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl ObjType {
/// The main SOM Object trait. Notice that code should almost never call these functions directly:
/// you should instead call the equivalent function in the `Val` struct.
#[narrowable_alloy(ThinObj)]
pub trait Obj: std::fmt::Debug + Send + Sync + FinalizerSafe {
pub trait Obj: std::fmt::Debug + Send + Sync {
/// What `ObjType` does this `Val` represent?
fn dyn_objtype(self: Gc<Self>) -> ObjType;

Expand Down
5 changes: 0 additions & 5 deletions src/lib/vm/objects/string_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ pub struct String_ {
s: String,
}

// This is safe because there is no non-GC'd shared ownership inside `String_`.
// This means that even though a finalizer will call its drop methods in another
// thread, it is guaranteed that the finalizer thread will be the only thread
// accessing its data.
unsafe impl FinalizerSafe for String_ {}
unsafe impl Sync for String_ {}

impl Obj for String_ {
Expand Down

0 comments on commit c0a71d6

Please sign in to comment.