[context] Use traits to control whether collectors are 'Sync'

No longer dependent on features, although the 'sync' feature is still
present to optionally remove our dependency on parking_lot.
Even if the feature is on, you still have the option for single-threaded
collectors if you want to.

Author: Techcable

libs/context/Cargo.toml

@@ -19,12 +19,13 @@ slog = "2.7"
 
 [features]
 default = [
-    "sync", # Thread-safety by default
+    "sync", # Support thread-safety by default
 ]
-# Allow multiple threads to access the garbage collector
+# This will allow multiple threads to access the garbage collector
 # by creating a seperate context for each.
 #
-# This can increase overhead by requiring communication between threads.
+# Thread safe collectors can have increased overhead
+# by requiring communication between threads.
 sync = [
     "parking_lot",
     "crossbeam-utils"

libs/context/src/collector.rs

@@ -9,7 +9,8 @@ use slog::{Logger, o};
 
 use zerogc::{Gc, GcSafe, GcSystem, Trace, GcSimpleAlloc};
 
-use crate::{CollectorContext, CollectionManager};
+use crate::{CollectorContext};
+use crate::state::{CollectionManager, RawContext};
 
 /// A specific implementation of a collector
 pub unsafe trait RawCollectorImpl: 'static + Sized {
@@ -18,17 +19,27 @@ pub unsafe trait RawCollectorImpl: 'static + Sized {
     /// The simple collector implements this as
     /// a trait object pointer.
     type GcDynPointer: Copy + Debug + 'static;
+
     /// A pointer to this collector
     ///
     /// Must be a ZST if the collector is a singleton.
     type Ptr: CollectorPtr<Self>;
 
+    /// The type that manages this collector's state
+    type Manager: CollectionManager<Self, Context=Self::RawContext>;
+
+    /// The context
+    type RawContext: RawContext<Self>;
+
     /// True if this collector is a singleton
     ///
     /// If the collector allows multiple instances,
     /// this *must* be false
     const SINGLETON: bool;
 
+    /// True if this collector is thread-safe.
+    const SYNC: bool;
+
     /// Convert the specified value into a dyn pointer
     unsafe fn create_dyn_pointer<T: Trace>(t: *mut T) -> Self::GcDynPointer;
 
@@ -50,14 +61,20 @@ pub unsafe trait RawCollectorImpl: 'static + Sized {
     /// The logger associated with this collector
     fn logger(&self) -> &Logger;
 
-    fn manager(&self) -> &CollectionManager<Self>;
+    fn manager(&self) -> &Self::Manager;
 
     fn should_collect(&self) -> bool;
 
     fn allocated_size(&self) -> crate::utils::MemorySize;
 
-    unsafe fn perform_raw_collection(&self, contexts: &[*mut crate::RawContext<Self>]);
+    unsafe fn perform_raw_collection(&self, contexts: &[*mut Self::RawContext]);
 }
+
+/// A thread safe collector
+pub unsafe trait SyncCollector: RawCollectorImpl + Sync {
+
+}
+
 /// A collector implemented as a singleton
 ///
 /// This only has one instance
@@ -322,10 +339,9 @@ pub struct CollectorRef<C: RawCollectorImpl> {
     ptr: C::Ptr
 }
 /// We actually are thread safe ;)
+unsafe impl<C: SyncCollector> Send for CollectorRef<C> {}
 #[cfg(feature = "sync")]
-unsafe impl<C: RawCollectorImpl + Sync> Send for CollectorRef<C> {}
-#[cfg(feature = "sync")]
-unsafe impl<C: RawCollectorImpl + Sync> Sync for CollectorRef<C> {}
+unsafe impl<C: SyncCollector> Sync for CollectorRef<C> {}
 
 /// Internal trait for initializing a collector
 #[doc(hidden)]
@@ -384,24 +400,23 @@ impl<C: RawCollectorImpl> CollectorRef<C> {
         CollectorId { ptr: self.ptr  }
     }
 
+    /// Convert this collector into a unique context
+    ///
+    /// The single-threaded implementation only allows a single context,
+    /// so this method is nessicary to support it.
+    pub fn into_context(self) -> CollectorContext<C> {
+        unsafe { CollectorContext::register_root(&self) }
+    }
+}
+impl<C: SyncCollector> CollectorRef<C> {
+
     /// Create a new context bound to this collector
     ///
     /// Warning: Only one collector should be created per thread.
     /// Doing otherwise can cause deadlocks/panics.
-    #[cfg(feature = "sync")]
     pub fn create_context(&self) -> CollectorContext<C> {
         unsafe { CollectorContext::register_root(&self) }
     }
-    /// Convert this collector into a unique context
-    ///
-    /// The single-threaded implementation only allows a single context,
-    /// so this method is nessicarry to support it.
-    pub fn into_context(self) -> CollectorContext<C> {
-        #[cfg(feature = "sync")]
-            { self.create_context() }
-        #[cfg(not(feature = "sync"))]
-            unsafe { CollectorContext::from_collector(&self) }
-    }
 }
 impl<C: RawCollectorImpl> Drop for CollectorRef<C> {
     #[inline]

libs/context/src/handle.rs

@@ -6,7 +6,7 @@ use std::marker::PhantomData;
 use std::sync::atomic::{self, AtomicPtr, AtomicUsize, Ordering};
 
 use zerogc::{Trace, GcSafe, GcBrand, GcVisitor, NullTrace, TraceImmutable, GcHandleSystem, GcBindHandle};
-use crate::{Gc, WeakCollectorRef, CollectorId, CollectorContext, CollectorRef};
+use crate::{Gc, WeakCollectorRef, CollectorId, CollectorContext, CollectorRef, CollectionManager};
 use crate::collector::RawCollectorImpl;
 
 const INITIAL_HANDLE_CAPACITY: usize = 64;
@@ -384,11 +384,8 @@ unsafe impl<T: GcSafe, C: RawHandleImpl> ::zerogc::GcHandle<T> for GcHandle<T, C
     type System = CollectorRef<C>;
     type Id = CollectorId<C>;
 
-    #[cfg(feature = "sync")]
     fn use_critical<R>(&self, func: impl FnOnce(&T) -> R) -> R {
         self.collector.ensure_valid(|collector| unsafe {
-            // Used for 'prevent_collection' method
-            use crate::SyncCollectorImpl;
             /*
              * This should be sufficient to ensure
              * the value won't be collected or relocated.
@@ -398,17 +395,13 @@ unsafe impl<T: GcSafe, C: RawHandleImpl> ::zerogc::GcHandle<T> for GcHandle<T, C
              * This is preferable to using `recursive_read`,
              * since that could starve writers (collectors).
              */
-            collector.as_ref().prevent_collection(|_state| {
+            C::Manager::prevent_collection(collector.as_ref(), || {
                 let value = self.inner.as_ref().value
                     .load(Ordering::Acquire) as *mut T;
                 func(&*value)
             })
         })
     }
-    #[cfg(not(feature = "sync"))]
-    fn use_critical<R>(&self, _func: impl FnOnce(&T) -> R) -> R {
-        unimplemented!("critical sections for single-collector impl")
-    }
 }
 unsafe impl<'new_gc, T, C> GcBindHandle<'new_gc, T> for GcHandle<T, C>
     where T: GcSafe, T: GcBrand<'new_gc, CollectorId<C>>,

libs/context/src/lib.rs

@@ -10,26 +10,16 @@ use std::fmt::{self, Debug, Formatter};
 
 use zerogc::prelude::*;
 
-#[cfg(feature = "sync")]
-mod sync;
-#[cfg(not(feature = "sync"))]
-mod nosync;
-#[cfg(feature = "sync")]
-pub use self::sync::*;
-#[cfg(not(feature = "sync"))]
-pub use self::nosync::*;
+pub mod state;
 
 pub mod utils;
 pub mod collector;
 pub mod handle;
 
 use crate::collector::{RawCollectorImpl};
-#[cfg(feature = "sync")]
-use crate::sync::SyncCollectorImpl;
-#[cfg(not(feature = "sync"))]
-use crate::nosync::NoSyncCollectorImpl;
 
 pub use crate::collector::{WeakCollectorRef, CollectorRef, CollectorId};
+pub use crate::state::{CollectionManager, RawContext};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum ContextState {
@@ -100,52 +90,42 @@ impl ContextState {
  */
 // TODO: Rename to remove 'Simple' from name
 pub struct CollectorContext<C: RawCollectorImpl> {
-    raw: *mut RawContext<C>,
+    raw: *mut C::RawContext,
     /// Whether we are the root context
     ///
     /// Only the root actually owns the `Arc`
     /// and is responsible for dropping it
     root: bool
 }
 impl<C: RawCollectorImpl> CollectorContext<C> {
-    #[cfg(not(feature = "sync"))]
-    pub(crate) unsafe fn from_collector(collector: &CollectorRef<C>) -> Self {
-        CollectorContext {
-            raw: Box::into_raw(ManuallyDrop::into_inner(
-                RawContext::from_collector(collector.clone_internal())
-            )),
-            root: true // We are the exclusive owner
-        }
-    }
-    #[cfg(feature = "sync")]
     pub(crate) unsafe fn register_root(collector: &CollectorRef<C>) -> Self {
         CollectorContext {
             raw: Box::into_raw(ManuallyDrop::into_inner(
-                RawContext::register_new(&collector)
+                C::RawContext::register_new(&collector)
             )),
             root: true, // We are responsible for unregistering
         }
     }
     #[inline]
     pub fn collector(&self) -> &C {
-        unsafe { &(*self.raw).collector.as_raw() }
+        unsafe { &(*self.raw).collector() }
     }
     #[inline(always)]
     unsafe fn with_shadow_stack<R, T: Trace>(
         &self, value: *mut &mut T, func: impl FnOnce() -> R
     ) -> R {
-        let old_link = (*(*self.raw).shadow_stack.get()).last;
+        let old_link = (*(*self.raw).shadow_stack_ptr()).last;
         let new_link = ShadowStackLink {
             element: C::create_dyn_pointer(value),
             prev: old_link
         };
-        (*(*self.raw).shadow_stack.get()).last = &new_link;
+        (*(*self.raw).shadow_stack_ptr()).last = &new_link;
         let result = func();
         debug_assert_eq!(
-            (*(*self.raw).shadow_stack.get()).last,
+            (*(*self.raw).shadow_stack_ptr()).last,
             &new_link
         );
-        (*(*self.raw).shadow_stack.get()).last = new_link.prev;
+        (*(*self.raw).shadow_stack_ptr()).last = new_link.prev;
         result
     }
     #[cold]
@@ -160,7 +140,7 @@ impl<C: RawCollectorImpl> Drop for CollectorContext<C> {
     fn drop(&mut self) {
         if self.root {
             unsafe {
-                self.collector().free_context(self.raw);
+                C::Manager::free_context(self.collector(), self.raw);
             }
         }
     }
@@ -171,25 +151,25 @@ unsafe impl<C: RawCollectorImpl> GcContext for CollectorContext<C> {
 
     #[inline]
     unsafe fn basic_safepoint<T: Trace>(&mut self, value: &mut &mut T) {
-        debug_assert_eq!((*self.raw).state.get(), ContextState::Active);
-        if (*self.raw).collector.as_raw().should_collect() {
+        debug_assert_eq!((*self.raw).state(), ContextState::Active);
+        if (*self.raw).collector().should_collect() {
             self.trigger_basic_safepoint(value);
         }
-        debug_assert_eq!((*self.raw).state.get(), ContextState::Active);
+        debug_assert_eq!((*self.raw).state(), ContextState::Active);
     }
 
     unsafe fn freeze(&mut self) {
-        (*self.raw).collector.as_raw().manager().freeze_context(&*self.raw);
+        (*self.raw).collector().manager().freeze_context(&*self.raw);
     }
 
     unsafe fn unfreeze(&mut self) {
-        (*self.raw).collector.as_raw().manager().unfreeze_context(&*self.raw);
+        (*self.raw).collector().manager().unfreeze_context(&*self.raw);
     }
 
     #[inline]
     unsafe fn recurse_context<T, F, R>(&self, value: &mut &mut T, func: F) -> R
         where T: Trace, F: for<'gc> FnOnce(&'gc mut Self, &'gc mut T) -> R {
-        debug_assert_eq!((*self.raw).state.get(), ContextState::Active);
+        debug_assert_eq!((*self.raw).state(), ContextState::Active);
         self.with_shadow_stack(value, || {
             let mut sub_context = ManuallyDrop::new(CollectorContext {
                 /*
@@ -204,7 +184,7 @@ unsafe impl<C: RawCollectorImpl> GcContext for CollectorContext<C> {
             debug_assert!(!sub_context.root);
             // No need to run drop code on context.....
             std::mem::forget(sub_context);
-            debug_assert_eq!((*self.raw).state.get(), ContextState::Active);
+            debug_assert_eq!((*self.raw).state(), ContextState::Active);
             result
         })
     }

libs/context/src/state/mod.rs

@@ -0,0 +1,78 @@
+use crate::collector::RawCollectorImpl;
+use crate::{ContextState, ShadowStack, CollectorRef};
+use std::mem::ManuallyDrop;
+use std::fmt::Debug;
+
+/// The internal state of the collector
+///
+/// Has a thread-safe and thread-unsafe implementation.
+
+#[cfg(feature = "sync")]
+pub mod sync;
+pub mod nosync;
+
+/// Manages coordination of garbage collections
+pub unsafe trait CollectionManager<C>: self::sealed::Sealed
+    where C: RawCollectorImpl<Manager=Self, RawContext=Self::Context> {
+    type Context: RawContext<C>;
+    fn new() -> Self;
+    fn is_collecting(&self) -> bool;
+    fn should_trigger_collection(&self) -> bool;
+    unsafe fn freeze_context(&self, context: &Self::Context);
+    unsafe fn unfreeze_context(&self, context: &Self::Context);
+
+    //
+    // Extension methods on collector
+    //
+
+    /// Attempt to prevent garbage collection for the duration of the closure
+    ///
+    /// This method is **OPTIONAL** and will panic if unimplemented.
+    fn prevent_collection<R>(collector: &C, func: impl FnOnce() -> R) -> R;
+
+    /// Free the specified context
+    ///
+    /// ## Safety
+    /// - Assumes the specified pointer is valid
+    /// - Assumes there are no more outstanding borrows to values in the context
+    unsafe fn free_context(collector: &C, context: *mut Self::Context);
+}
+
+/// The underlying state of a context
+///
+/// Each context is bound to one and only one thread,
+/// even if the collector supports multi-threading.
+pub unsafe trait RawContext<C>: Debug + self::sealed::Sealed
+    where C: RawCollectorImpl<RawContext=Self> {
+    unsafe fn register_new(collector: &CollectorRef<C>) -> ManuallyDrop<Box<Self>>;
+    /// Trigger a safepoint for this context.
+    ///
+    /// This implicitly attempts a collection,
+    /// potentially blocking until completion..
+    ///
+    /// Undefined behavior if mutated during collection
+    unsafe fn trigger_safepoint(&self);
+    /// Borrow a reference to the shadow stack,
+    /// assuming this context is valid (not active).
+    ///
+    /// A context is valid if it is either frozen
+    /// or paused at a safepoint.
+    #[inline]
+    unsafe fn assume_valid_shadow_stack(&self) -> &ShadowStack<C> {
+        match self.state() {
+            ContextState::Active => unreachable!("active context: {:?}", self),
+            ContextState::SafePoint { .. } | ContextState::Frozen { .. } => {},
+        }
+        &*self.shadow_stack_ptr()
+    }
+    /// Get a pointer to the shadow stack
+    fn shadow_stack_ptr(&self) -> *mut ShadowStack<C>;
+    /// Get a reference to the collector,
+    /// assuming that it's valid
+    unsafe fn collector(&self) -> &C;
+    fn state(&self) -> ContextState;
+}
+
+mod sealed {
+    pub trait Sealed {}
+}