Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Gc::new_cyclic #3292

Merged
merged 2 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions boa_gc/src/internals/ephemeron_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ struct Data<K: Trace + 'static, V: Trace + 'static> {
}

impl<K: Trace, V: Trace> EphemeronBox<K, V> {
/// Creates a new `EphemeronBox` that tracks `key` and has `value` as its inner data.
pub(crate) fn new(key: &Gc<K>, value: V) -> Self {
Self {
header: EphemeronBoxHeader::new(),
Expand All @@ -110,6 +111,14 @@ impl<K: Trace, V: Trace> EphemeronBox<K, V> {
}
}

/// Creates a new `EphemeronBox` with its inner data in the invalidated state.
pub(crate) fn new_empty() -> Self {
Self {
header: EphemeronBoxHeader::new(),
data: UnsafeCell::new(None),
}
}

/// Returns `true` if the two references refer to the same `EphemeronBox`.
pub(crate) fn ptr_eq(this: &Self, other: &Self) -> bool {
// Use .header to ignore fat pointer vtables, to work around
Expand All @@ -121,8 +130,8 @@ impl<K: Trace, V: Trace> EphemeronBox<K, V> {
///
/// # Safety
///
/// The garbage collector must not run between the call to this function and the eventual
/// drop of the returned reference, since that could free the inner value.
/// The caller must ensure there are no live mutable references to the ephemeron box's data
/// before calling this method.
pub(crate) unsafe fn value(&self) -> Option<&V> {
// SAFETY: the garbage collector ensures the ephemeron doesn't mutate until
// finalization.
Expand All @@ -134,8 +143,8 @@ impl<K: Trace, V: Trace> EphemeronBox<K, V> {
///
/// # Safety
///
/// The garbage collector must not run between the call to this function and the eventual
/// drop of the returned reference, since that could free the inner value.
/// The caller must ensure there are no live mutable references to the ephemeron box's data
/// before calling this method.
pub(crate) unsafe fn key(&self) -> Option<&GcBox<K>> {
// SAFETY: the garbage collector ensures the ephemeron doesn't mutate until
// finalization.
Expand All @@ -153,6 +162,22 @@ impl<K: Trace, V: Trace> EphemeronBox<K, V> {
self.header.mark();
}

/// Sets the inner data of the `EphemeronBox` to the specified key and value.
///
/// # Safety
///
/// The caller must ensure there are no live mutable references to the ephemeron box's data
/// before calling this method.
pub(crate) unsafe fn set(&self, key: &Gc<K>, value: V) {
// SAFETY: The caller must ensure setting the key and value of the ephemeron box is safe.
unsafe {
*self.data.get() = Some(Data {
key: key.inner_ptr(),
value,
});
}
}

#[inline]
pub(crate) fn inc_ref_count(&self) {
self.header.ref_count.set(self.header.ref_count.get() + 1);
Expand Down
3 changes: 2 additions & 1 deletion boa_gc/src/pointers/ephemeron.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ impl<K: Trace, V: Trace + Clone> Ephemeron<K, V> {

impl<K: Trace, V: Trace> Ephemeron<K, V> {
/// Creates a new `Ephemeron`.
#[must_use]
pub fn new(key: &Gc<K>, value: V) -> Self {
let inner_ptr = Allocator::alloc_ephemeron(EphemeronBox::new(key, value));
Self { inner_ptr }
Expand Down Expand Up @@ -72,7 +73,7 @@ impl<K: Trace, V: Trace> Ephemeron<K, V> {
/// This function is unsafe because improper use may lead to memory corruption, double-free,
/// or misbehaviour of the garbage collector.
#[must_use]
const unsafe fn from_raw(inner_ptr: NonNull<EphemeronBox<K, V>>) -> Self {
pub(crate) const unsafe fn from_raw(inner_ptr: NonNull<EphemeronBox<K, V>>) -> Self {
Self { inner_ptr }
}
}
Expand Down
34 changes: 32 additions & 2 deletions boa_gc/src/pointers/gc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::{
finalizer_safe,
internals::GcBox,
internals::{EphemeronBox, GcBox},
trace::{Finalize, Trace},
Allocator,
Allocator, Ephemeron, WeakGc,
};
use std::{
cmp::Ordering,
Expand All @@ -22,6 +22,7 @@ pub struct Gc<T: Trace + ?Sized + 'static> {

impl<T: Trace> Gc<T> {
/// Constructs a new `Gc<T>` with the given value.
#[must_use]
pub fn new(value: T) -> Self {
// Create GcBox and allocate it to heap.
//
Expand All @@ -34,6 +35,35 @@ impl<T: Trace> Gc<T> {
}
}

/// Constructs a new `Gc<T>` while giving you a `WeakGc<T>` to the allocation, to allow
/// constructing a T which holds a weak pointer to itself.
///
/// Since the new `Gc<T>` is not fully-constructed until `Gc<T>::new_cyclic` returns, calling
/// [`upgrade`][WeakGc::upgrade] on the weak reference inside the closure will fail and result
/// in a `None` value.
#[must_use]
pub fn new_cyclic<F>(data_fn: F) -> Self
where
F: FnOnce(&WeakGc<T>) -> T,
{
// SAFETY: The newly allocated ephemeron is only live here, meaning `Ephemeron` is the
// sole owner of the allocation after passing it to `from_raw`, making this operation safe.
let weak = unsafe {
Ephemeron::from_raw(Allocator::alloc_ephemeron(EphemeronBox::new_empty())).into()
};

let gc = Self::new(data_fn(&weak));

// SAFETY:
// - `as_mut`: `weak` is properly initialized by `alloc_ephemeron` and cannot escape the
// `unsafe` block.
// - `set_kv`: `weak` is a newly created `EphemeronBox`, meaning it isn't possible to
// collect it since `weak` is still live.
unsafe { weak.inner().inner_ptr().as_mut().set(&gc, gc.clone()) }

gc
}

/// Consumes the `Gc`, returning a wrapped raw pointer.
///
/// To avoid a memory leak, the pointer must be converted back to a `Gc` using [`Gc::from_raw`].
Expand Down
8 changes: 7 additions & 1 deletion boa_gc/src/pointers/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ impl<T: Trace> WeakGc<T> {
}
}

/// Upgrade returns a `Gc` pointer for the internal value if valid, or None if the value was already garbage collected.
/// Upgrade returns a `Gc` pointer for the internal value if the pointer is still live, or `None`
/// if the value was already garbage collected.
#[must_use]
pub fn upgrade(&self) -> Option<Gc<T>> {
self.inner.value()
Expand All @@ -31,6 +32,11 @@ impl<T: Trace> WeakGc<T> {
pub fn is_upgradable(&self) -> bool {
self.inner.has_value()
}

#[must_use]
pub(crate) const fn inner(&self) -> &Ephemeron<T, Gc<T>> {
&self.inner
}
}

impl<T: Trace> Clone for WeakGc<T> {
Expand Down
Loading