Skip to content

Commit

Permalink
WIP: Unsized types (#45)
Browse files Browse the repository at this point in the history
* Add ?Sized bounds to all (?) appropriate locations

I have added ?Sized to pretty much all T that is garbage collected and
that needs it. However, there are a few structs defined in a rental!
macro that are causing the build to break due to trasmutation rules.
Enforcing a Sized bound causes the GcRef type to require a Sized T,
which doesn't seem right.

* Removed ?Sized bounds from smart-pointer implementations of RwLock, Mutex, and RefCell

The implementations of RwLock, Mutex, and RefCell all have special
guards for their borrow() (and related) functions that wrap around their
normal standard library guards. These guards are implemented using the
rental crate, which has a special macro that does some magic behind the
scenes involving transmutation of types - which breaks some type rules
when they're possibly unsized.

This now compiles, and the Gc<RwLock>, Gc<Mutex>, and Gc<RefCell> types
do not have access to the ergonomic GcRef type or their special borrow()
family of methods.

* WIP: add nightly-only features CoerceUnsized and Unsize

These allow a `Gc<T>` to be coerced to a `Gc<dyn R>` when `T` is some
trait `R`. This requires `std::ops::CoerceUnsized` and
`std::marker::Unsize`, which are nightly features.

TODO: Add compile-time bounds for these functions/imports

* Add support for converting Box<T> to a Gc<T> where T: ?Size

Unsized types are able to be created by allocating a Box<T>, converting
it to a Box<dyn MyTrait>, and then moving that pointer to the allocator.

* Add `ToScan` trait which forces the value to be converted to a `&dyn
  Scan`, which is required for using any of the new `from_box` methods.
  Also add `impl_to_scan_safe` macro for convenience
* Add Gc::from_box function for converting the boxed value to a GC'd
  value
* Add GcAllocation::from_box function which does the actual convertion
  from the boxed value to a pointer
* Add Colletor::track_boxed_value which tracks a pointer as if it is a
  raw pointer from a `Box<T>`
* Add DeallocationAction::BoxDrop, which tells the collector tracking to
  convert the associated pointer back to the appropriate `Box<T>` before
  dropping it

* Add guards to nightly features

CoerceUnsized and Unsize are both nightly features. These are now
guarded at compile time so the library will compile on stable Rust
again.

* Run cargo fmt

* Remove impl_to_scan_safe macro in favor of blanket impl

* Alphabetize new member in DeallocateAction with respect to other members

* Add newline for style

Signed-off-by: Alek Ratzloff <alekratz@gmail.com>

* Add exception for calling dealloc() in GcAllocation::deallocate()

Since types that are adapted from a boxed pointer are already dropped by
the time dealloc() is called, this would result in a double-free which
is !!!UNSAFE!!! and an obvious bug. Now, the deallocation action is
checked before attempting to call dealloc() on a pointer.

* Add "nightly-features" feature, to enable nightly features

It appears my use of `#[cfg(nightly)]` was futile, since that's not an
actual flag that the compiler uses with nightly code. Basically, users
have to opt-in to nightly features themselves.

Now that this has been fixed, a couple of compile-time errors popped up
that are fixed as well.

* Add ?Sized constraint for T on impl CoerceUnsized for Gc<T>

This was preventing basic operation (i.e. transitivity was not being
upheld). Weird errors for sure.

* Add small integration test for coercion of unsized types

This doesn't really test anything extensive, it mostly demonstrates how
unsized types may be used with linked lists.

* Remove useless test mutex from coercion test

There is only one test in this file, so there are no race conditions to
guard against.

* Add test for unsized (immutable) types

This test ensures that some `T` that is `SomeTrait: Scan + ToScan` can be
put behind a `Gc<dyn SomeTrait>` pointer and shared that way. This tests
`Gc::from_box`, which is the primary way to access this behavior.

* Add make_node! macro to unsized type test

* Run rustfmt
  • Loading branch information
alekratz authored Aug 21, 2020
1 parent 8a21da4 commit 419a34b
Show file tree
Hide file tree
Showing 10 changed files with 398 additions and 41 deletions.
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,7 @@ trybuild = "1.0"

#[profile.release]
#debug = true

[features]
default = []
nightly-features = []
35 changes: 31 additions & 4 deletions src/collector/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::panic::UnwindSafe;
use std::ptr;

use crate::collector::InternalGcRef;
use crate::{Finalize, Scan, Scanner};
use crate::{Finalize, Scan, Scanner, ToScan};

/// Represents a piece of data allocated by shredder
#[derive(Copy, Clone, Debug, Hash)]
Expand All @@ -16,6 +16,7 @@ pub struct GcAllocation {
/// What additional action should we run before deallocating?
#[derive(Copy, Clone, Debug, Hash)]
pub enum DeallocationAction {
BoxDrop,
DoNothing,
RunDrop,
RunFinalizer { finalize_ptr: *const dyn Finalize },
Expand Down Expand Up @@ -67,6 +68,19 @@ impl GcAllocation {
)
}

pub fn from_box<T: Scan + ToScan + ?Sized + 'static>(v: Box<T>) -> (Self, *const T) {
let scan_ptr: *const dyn Scan = v.to_scan();
let raw_ptr: *const T = Box::into_raw(v);

(
Self {
scan_ptr,
deallocation_action: DeallocationAction::BoxDrop,
},
raw_ptr,
)
}

#[allow(clippy::transmute_ptr_to_ptr)]
fn raw_allocate<'a, T: Scan + 'a>(v: T) -> (*const dyn Scan, *const T) {
// This is a straightforward use of alloc/write -- it should be undef free
Expand Down Expand Up @@ -121,11 +135,24 @@ impl GcAllocation {
// So we can run `finalize` here, right before deallocation
(&mut *(finalize_ptr as *mut dyn Finalize)).finalize();
}
DeallocationAction::BoxDrop => {
// Safe as long as only boxed values are created with BoxDrop deallocate action
// Additionally, this is the only instance where the pointer should be alive so
// we are not taking it mutably anywhere else. The death of a pointer in action,
// really makes you think...
let box_ptr = Box::from_raw(scan_ptr as *mut dyn Scan);
// drop like normal
drop(box_ptr);
}
}

let dealloc_layout = Layout::for_value(&*scan_ptr);
let heap_ptr = scan_ptr as *mut u8;
dealloc(heap_ptr, dealloc_layout);
// Only call dealloc() if we're not dealing with a boxed value, because the box gets
// dropped above.
if !matches!(self.deallocation_action, DeallocationAction::BoxDrop) {
let dealloc_layout = Layout::for_value(&*scan_ptr);
let heap_ptr = scan_ptr as *mut u8;
dealloc(heap_ptr, dealloc_layout);
}
}

pub fn scan<F: FnMut(InternalGcRef)>(&self, callback: F) {
Expand Down
12 changes: 10 additions & 2 deletions src/collector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub use crate::collector::atomic_protection::*;
use crate::collector::dropper::{BackgroundDropper, DropMessage};
use crate::collector::trigger::GcTrigger;
use crate::lockout::{ExclusiveWarrant, Lockout, LockoutProvider, Warrant};
use crate::{Finalize, Scan};
use crate::{Finalize, Scan, ToScan};

/// Intermediate struct. `Gc<T>` holds a `InternalGcRef`, which references a `GcHandle`
/// There should be one `GcHandle` per `Gc<T>`
Expand Down Expand Up @@ -274,7 +274,15 @@ impl Collector {
self.track(gc_data_ptr, heap_ptr)
}

fn track<T: Scan>(
pub fn track_boxed_value<T: Scan + ToScan + ?Sized + 'static>(
&self,
data: Box<T>,
) -> (InternalGcRef, *const T) {
let (gc_data_ptr, heap_ptr) = GcAllocation::from_box(data);
self.track(gc_data_ptr, heap_ptr)
}

fn track<T: Scan + ?Sized>(
&self,
gc_data_ptr: GcAllocation,
heap_ptr: *const T,
Expand Down
2 changes: 1 addition & 1 deletion src/finalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub unsafe trait Finalize {
unsafe fn finalize(&mut self);
}

unsafe impl<T: Scan> Finalize for Gc<T> {
unsafe impl<T: Scan + ?Sized> Finalize for Gc<T> {
unsafe fn finalize(&mut self) {
self.internal_handle().invalidate();
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
//! - further parallelization: The collector needs to be optimized and parallelized further (will fix!)
//! - no no-std support: The collector requires threading and other `std` features (will fix!)
#![cfg_attr(feature = "nightly-features", feature(unsize, coerce_unsized))]
// We love docs here
#![deny(missing_docs)]
// Clippy configuration:
Expand Down Expand Up @@ -65,7 +66,7 @@ use std::sync::{Mutex, RwLock};
use collector::COLLECTOR;

pub use finalize::Finalize;
pub use scan::{EmptyScan, GcSafe, GcSafeWrapper, RMut, Scan, Scanner, R};
pub use scan::{EmptyScan, GcSafe, GcSafeWrapper, RMut, Scan, Scanner, ToScan, R};
pub use smart_ptr::{Gc, GcGuard};

// Re-export the Scan derive
Expand Down
23 changes: 20 additions & 3 deletions src/scan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,23 @@ macro_rules! mark_send_type_gc_safe {
};
}

/// A trait that allows something that is `Scan` to be converted to a `dyn` ref.
///
/// Implementing this trait is only necessary if you need to allocate an owned pointer to a DST,
/// e.g. `Gc::from_box(Box<dyn MyTrait>)`
///
/// This is unsafe because `to_scan` must always be implemented as `&*self`
pub unsafe trait ToScan {
/// Converts this value to a `dyn Scan` reference value.
fn to_scan(&self) -> &(dyn Scan + 'static);
}

unsafe impl<T: Scan + Sized + 'static> ToScan for T {
fn to_scan(&self) -> &(dyn Scan + 'static) {
&*self
}
}

/// Scanner is a struct used to manage the scanning of data, sort of analogous to `Hasher`
/// Usually you will only care about this while implementing `Scan`
pub struct Scanner<'a> {
Expand All @@ -133,7 +150,7 @@ impl<'a> Scanner<'a> {
}

/// Scan a piece of data, tracking any `Gc`s found
pub fn scan<T: Scan>(&mut self, from: &T) {
pub fn scan<T: Scan + ?Sized>(&mut self, from: &T) {
from.scan(self);
}

Expand All @@ -149,14 +166,14 @@ impl<'a> Scanner<'a> {

// This is a fundamental implementation, since it's how GcInternalHandles make it into the Scanner
// Safety: The implementation is built around this, so it's by definition safe
unsafe impl<T: Scan> Scan for Gc<T> {
unsafe impl<T: Scan + ?Sized> Scan for Gc<T> {
#[allow(clippy::inline_always)]
#[inline(always)]
fn scan(&self, scanner: &mut Scanner<'_>) {
scanner.add_internal_handle(self.internal_handle());
}
}
unsafe impl<T: Scan> GcSafe for Gc<T> {}
unsafe impl<T: Scan + ?Sized> GcSafe for Gc<T> {}

// Another fundamental implementation
unsafe impl<T: Scan> Scan for AtomicGc<T> {
Expand Down
13 changes: 7 additions & 6 deletions src/scan/std_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ where
}
unsafe impl<T: GcSafe> GcSafe for Cell<T> {}

unsafe impl<T: Scan> Scan for RefCell<T> {
unsafe impl<T: Scan + ?Sized> Scan for RefCell<T> {
#[inline]
fn scan(&self, scanner: &mut Scanner<'_>) {
// It's an error if this fails
Expand All @@ -89,7 +89,8 @@ unsafe impl<T: Scan> Scan for RefCell<T> {
}
}
}
unsafe impl<T: GcSafe> GcSafe for RefCell<T> {}

unsafe impl<T: GcSafe + ?Sized> GcSafe for RefCell<T> {}

unsafe impl<T: Scan> Scan for Option<T> {
#[inline]
Expand All @@ -101,7 +102,7 @@ unsafe impl<T: Scan> Scan for Option<T> {
}
unsafe impl<T: GcSafe> GcSafe for Option<T> {}

unsafe impl<T: Scan> Scan for Mutex<T> {
unsafe impl<T: Scan + ?Sized> Scan for Mutex<T> {
#[inline]
fn scan(&self, scanner: &mut Scanner<'_>) {
match self.try_lock() {
Expand All @@ -120,9 +121,9 @@ unsafe impl<T: Scan> Scan for Mutex<T> {
}
}
}
unsafe impl<T: GcSafe> GcSafe for Mutex<T> {}
unsafe impl<T: GcSafe + ?Sized> GcSafe for Mutex<T> {}

unsafe impl<T: Scan> Scan for RwLock<T> {
unsafe impl<T: Scan + ?Sized> Scan for RwLock<T> {
#[inline]
fn scan(&self, scanner: &mut Scanner<'_>) {
match self.try_read() {
Expand All @@ -141,7 +142,7 @@ unsafe impl<T: Scan> Scan for RwLock<T> {
}
}
}
unsafe impl<T: GcSafe> GcSafe for RwLock<T> {}
unsafe impl<T: GcSafe + ?Sized> GcSafe for RwLock<T> {}

// Primitives do not hold any Gc<T>s
impl EmptyScan for isize {}
Expand Down
Loading

0 comments on commit 419a34b

Please sign in to comment.