diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml index d7d16ee..3ec2b54 100644 --- a/.github/workflows/miri.yml +++ b/.github/workflows/miri.yml @@ -7,7 +7,7 @@ env: CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse jobs: - test-with-miri: + test-with-miri-stable: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -24,5 +24,19 @@ jobs: - name: Run tests # Keep "std" feature always enabled here to avoid needing the no-std related nightly features run: cargo hack miri test --feature-powerset --skip nightly,derive --verbose -F std,pedantic-debug-assertions + test-with-miri-nightly: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + # From Miri documentation: https://github.com/rust-lang/miri#running-miri-on-ci + - name: Install Miri + run: | + rustup toolchain install nightly --component miri + rustup override set nightly + cargo miri setup + - uses: taiki-e/install-action@cargo-hack + # Always skip "derive" since derive macro tests are skipped on Miri + # Also always keep "pedantic-debug-assertions" enabled to reduce build times + # Note: no need to use --workspace here, since there's no unsafe in rust-cc-derive - name: Run tests (nightly) run: cargo hack miri test --feature-powerset --skip derive --verbose -F nightly,pedantic-debug-assertions diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f9d224c..4ec4f4a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -41,7 +41,13 @@ Therefore, they undergo two tracing passes: Note that this second phase is correct only if the graph formed by the pointers is not changed between the two phases. Thus, this is a key requirement of the `Trace` trait and one of the reasons it is marked `unsafe`. -# Writing documentation +## Writing tests + +Every unit test should start with a call to `tests::reset_state()` to make sure errors in other tests don't impact the current one. + +Also, functions marked as `pub(crate)` and used only in unit tests should have the `for_tests` suffix, like `Cc::new_for_tests`. + +## Writing documentation Docs are always built with every feature enabled. This makes it easier to write and maintain the documentation. diff --git a/Cargo.toml b/Cargo.toml index 83e3e83..af35b42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ edition.workspace = true members = ["derive"] [workspace.package] -version = "0.3.0" # Also update in [dependencies.rust-cc-derive.version] +version = "0.4.0" # Also update in [dependencies.rust-cc-derive.version] authors = ["fren_gor "] repository = "https://github.com/frengor/rust-cc" categories = ["memory-management", "no-std"] @@ -37,10 +37,10 @@ auto-collect = [] finalization = [] # Enables weak pointers -weak-ptr = [] +weak-ptrs = [] # Enables cleaners -cleaners = ["dep:slotmap", "weak-ptr"] +cleaners = ["dep:slotmap", "weak-ptrs"] # Enables support for stdlib, disable for no-std support (requires ELF TLS and nightly) std = ["slotmap?/std", "thiserror/std"] @@ -49,7 +49,7 @@ std = ["slotmap?/std", "thiserror/std"] pedantic-debug-assertions = [] [dependencies] -rust-cc-derive = { path = "./derive", version = "=0.3.0", optional = true } +rust-cc-derive = { path = "./derive", version = "=0.4.0", optional = true } slotmap = { version = "1.0", optional = true } thiserror = { version = "1.0", package = "thiserror-core", default-features = false } @@ -64,6 +64,9 @@ name = "bench" harness = false required-features = ["std", "derive"] +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(doc_auto_cfg)'] } + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "doc_auto_cfg", "--generate-link-to-definition"] diff --git a/README.md b/README.md index 6feda5d..80a05b8 100644 --- a/README.md +++ b/README.md @@ -63,11 +63,6 @@ impl Finalize for Data { } ``` -> [!NOTE] -> Finalization adds an overhead to each collection execution. Cleaners provide a faster alternative to finalization. -> -> *When possible*, it's suggested to prefer cleaners and disable finalization. - For more information read [the docs](https://docs.rs/rust-cc/latest/rust_cc/). ## The collection algorithm diff --git a/src/cc.rs b/src/cc.rs index d3127fb..3461350 100644 --- a/src/cc.rs +++ b/src/cc.rs @@ -1,10 +1,16 @@ use alloc::alloc::Layout; +use alloc::rc::Rc; use core::cell::UnsafeCell; use core::marker::PhantomData; use core::mem; use core::ops::Deref; use core::ptr::{self, drop_in_place, NonNull}; -use alloc::rc::Rc; +use core::borrow::Borrow; +use core::cell::Cell; +use core::fmt::{self, Debug, Display, Formatter, Pointer}; +use core::cmp::Ordering; +use core::hash::{Hash, Hasher}; +use core::panic::{RefUnwindSafe, UnwindSafe}; #[cfg(feature = "nightly")] use core::{ marker::Unsize, @@ -18,6 +24,8 @@ use crate::trace::{Context, ContextInner, Finalize, Trace}; use crate::list::ListMethods; use crate::utils::*; use crate::POSSIBLE_CYCLES; +#[cfg(feature = "weak-ptrs")] +use crate::weak::weak_counter_marker::WeakCounterMarker; /// A thread-local cycle collected pointer. /// @@ -36,7 +44,7 @@ where { } -impl Cc { +impl Cc { /// Creates a new `Cc`. /// /// # Collection @@ -48,7 +56,6 @@ impl Cc { /// # Panics /// /// Panics if the automatically-stared collection panics. - #[inline(always)] #[must_use = "newly created Cc is immediately dropped"] #[track_caller] pub fn new(t: T) -> Cc { @@ -59,7 +66,7 @@ impl Cc { } #[cfg(feature = "auto-collect")] - super::trigger_collection(); + super::trigger_collection(state); Cc { inner: CcBox::new(t, state), @@ -98,7 +105,7 @@ impl Cc { } } -impl Cc { +impl Cc { /// Returns `true` if the two [`Cc`]s point to the same allocation. This function ignores the metadata of `dyn Trait` pointers. #[inline] pub fn ptr_eq(this: &Cc, other: &Cc) -> bool { @@ -165,13 +172,13 @@ impl Cc { unsafe { self.inner.as_ref() } } - #[cfg(feature = "weak-ptr")] + #[cfg(feature = "weak-ptrs")] #[inline(always)] pub(crate) fn inner_ptr(&self) -> NonNull> { self.inner } - #[cfg(feature = "weak-ptr")] // Currently used only here + #[cfg(feature = "weak-ptrs")] // Currently used only here #[inline(always)] #[must_use] pub(crate) fn __new_internal(inner: NonNull>) -> Cc { @@ -182,7 +189,7 @@ impl Cc { } } -impl Clone for Cc { +impl Clone for Cc { /// Makes a clone of the [`Cc`] pointer. /// /// This creates another pointer to the same allocation, increasing the strong reference count. @@ -214,7 +221,7 @@ impl Clone for Cc { } } -impl Deref for Cc { +impl Deref for Cc { type Target = T; #[inline] @@ -231,7 +238,7 @@ impl Deref for Cc { } } -impl Drop for Cc { +impl Drop for Cc { fn drop(&mut self) { #[cfg(debug_assertions)] if state(|state| state.is_tracing()) { @@ -239,14 +246,14 @@ impl Drop for Cc { } #[inline] - fn decrement_counter(cc: &Cc) { + fn decrement_counter(cc: &Cc) { // Always decrement the counter let res = cc.counter_marker().decrement_counter(); debug_assert!(res.is_ok()); } #[inline] - fn handle_possible_cycle(cc: &Cc) { + fn handle_possible_cycle(cc: &Cc) { decrement_counter(cc); // We know that we're not part of either root_list or non_root_list, since the cc isn't traced @@ -255,7 +262,8 @@ impl Drop for Cc { // A CcBox can be marked traced only during collections while being into a list different than POSSIBLE_CYCLES. // In this case, no further action has to be taken, except decrementing the reference counter. - if self.counter_marker().is_traced() { + // Skip the rest of the code also when the value has already been dropped + if self.counter_marker().is_traced_or_dropped() { decrement_counter(self); return; } @@ -287,11 +295,13 @@ impl Drop for Cc { let _dropping_guard = replace_state_field!(dropping, true, state); let layout = self.inner().layout(); - // Set the object as dropped before dropping and deallocating it - // This feature is used only in weak pointers, so do this only if they're enabled - #[cfg(feature = "weak-ptr")] + #[cfg(feature = "weak-ptrs")] { - self.counter_marker().set_dropped(true); + // Set the object as dropped before dropping and deallocating it + // This feature is used only in weak pointers, so do this only if they're enabled + self.counter_marker().mark(Mark::Dropped); + + self.inner().drop_metadata(); } // SAFETY: we're the only one to have a pointer to this allocation @@ -314,7 +324,7 @@ impl Drop for Cc { } } -unsafe impl Trace for Cc { +unsafe impl Trace for Cc { #[inline] #[track_caller] fn trace(&self, ctx: &mut Context<'_>) { @@ -324,18 +334,14 @@ unsafe impl Trace for Cc { } } -impl Finalize for Cc {} +impl Finalize for Cc {} #[repr(C)] pub(crate) struct CcBox { next: UnsafeCell>>>, prev: UnsafeCell>>>, - #[cfg(feature = "nightly")] - vtable: DynMetadata, - - #[cfg(not(feature = "nightly"))] - fat_ptr: NonNull, + metadata: Cell, counter_marker: CounterMarker, _phantom: PhantomData>, // Make CcBox !Send and !Sync @@ -345,8 +351,7 @@ pub(crate) struct CcBox { elem: UnsafeCell, } -impl CcBox { - #[inline(always)] +impl CcBox { #[must_use] fn new(t: T, state: &State) -> NonNull> { let layout = Layout::new::>(); @@ -363,10 +368,7 @@ impl CcBox { CcBox { next: UnsafeCell::new(None), prev: UnsafeCell::new(None), - #[cfg(feature = "nightly")] - vtable: metadata(ptr.as_ptr() as *mut dyn InternalTrace), - #[cfg(not(feature = "nightly"))] - fat_ptr: NonNull::new_unchecked(ptr.as_ptr() as *mut dyn InternalTrace), + metadata: Metadata::new(ptr), counter_marker: CounterMarker::new_with_counter_to_one(already_finalized), _phantom: PhantomData, elem: UnsafeCell::new(t), @@ -376,7 +378,6 @@ impl CcBox { } } - #[inline(always)] #[cfg(all(test, feature = "std"))] // Only used in unit tests #[must_use] pub(crate) fn new_for_tests(t: T) -> NonNull> { @@ -384,7 +385,7 @@ impl CcBox { } } -impl CcBox { +impl CcBox { #[inline] pub(crate) fn get_elem(&self) -> &T { unsafe { &*self.elem.get() } @@ -404,12 +405,72 @@ impl CcBox { pub(crate) fn layout(&self) -> Layout { #[cfg(feature = "nightly")] { - self.vtable.layout() + self.vtable().vtable.layout() } #[cfg(not(feature = "nightly"))] unsafe { - Layout::for_value(self.fat_ptr.as_ref()) + Layout::for_value(self.vtable().fat_ptr.as_ref()) + } + } + + #[inline] + fn vtable(&self) -> VTable { + #[cfg(feature = "weak-ptrs")] + unsafe { + if self.counter_marker.has_allocated_for_metadata() { + self.metadata.get().boxed_metadata.as_ref().vtable + } else { + self.metadata.get().vtable + } + } + + #[cfg(not(feature = "weak-ptrs"))] + unsafe { + self.metadata.get().vtable + } + } + + #[cfg(feature = "weak-ptrs")] + #[inline] + pub(crate) fn get_or_init_metadata(&self) -> NonNull { + unsafe { + if self.counter_marker.has_allocated_for_metadata() { + self.metadata.get().boxed_metadata + } else { + let vtable = self.metadata.get().vtable; + let ptr = BoxedMetadata::new(vtable, WeakCounterMarker::new(true)); + self.metadata.set(Metadata { + boxed_metadata: ptr, + }); + self.counter_marker.set_allocated_for_metadata(true); + ptr + } + } + } + + /// # Safety + /// The metadata must have been allocated. + #[cfg(feature = "weak-ptrs")] + #[inline(always)] + pub(crate) unsafe fn get_metadata_unchecked(&self) -> NonNull { + self.metadata.get().boxed_metadata + } + + #[cfg(feature = "weak-ptrs")] + #[inline] + pub(crate) fn drop_metadata(&self) { + if self.counter_marker.has_allocated_for_metadata() { + unsafe { + let boxed = self.get_metadata_unchecked(); + if boxed.as_ref().weak_counter_marker.counter() == 0 { + // There are no weak pointers, deallocate the metadata + dealloc_other(boxed); + } else { + // There exist weak pointers, set the CcBox allocation not accessible + boxed.as_ref().weak_counter_marker.set_accessible(false); + } + } } } @@ -424,14 +485,14 @@ impl CcBox { } } -unsafe impl Trace for CcBox { +unsafe impl Trace for CcBox { #[inline(always)] fn trace(&self, ctx: &mut Context<'_>) { self.get_elem().trace(ctx); } } -impl Finalize for CcBox { +impl Finalize for CcBox { #[inline(always)] fn finalize(&self) { self.get_elem().finalize(); @@ -528,6 +589,13 @@ impl CcBox<()> { /// SAFETY: `drop_in_place` conditions must be true. #[inline] pub(super) unsafe fn drop_inner(ptr: NonNull) { + #[cfg(feature = "weak-ptrs")] + { + // Set the object as dropped before dropping it + // This feature is used only in weak pointers, so do this only if they're enabled + ptr.as_ref().counter_marker().mark(Mark::Dropped); + } + CcBox::get_traceable(ptr).as_mut().drop_elem(); } @@ -535,13 +603,13 @@ impl CcBox<()> { fn get_traceable(ptr: NonNull) -> NonNull { #[cfg(feature = "nightly")] unsafe { - let vtable = ptr.as_ref().vtable; + let vtable = ptr.as_ref().vtable().vtable; NonNull::from_raw_parts(ptr.cast(), vtable) } #[cfg(not(feature = "nightly"))] unsafe { - ptr.as_ref().fat_ptr + ptr.as_ref().vtable().fat_ptr } } @@ -660,6 +728,67 @@ impl CcBox<()> { } } +#[derive(Copy, Clone)] +union Metadata { + vtable: VTable, + #[cfg(feature = "weak-ptrs")] + boxed_metadata: NonNull, +} + +impl Metadata { + #[inline] + fn new(cc_box: NonNull>) -> Cell { + #[cfg(feature = "nightly")] + let vtable = VTable { + vtable: metadata(cc_box.as_ptr() as *mut dyn InternalTrace), + }; + + #[cfg(not(feature = "nightly"))] + let vtable = VTable { + fat_ptr: unsafe { // SAFETY: the ptr comes from a NotNull ptr + NonNull::new_unchecked(cc_box.as_ptr() as *mut dyn InternalTrace) + }, + }; + + Cell::new(Metadata { + vtable + }) + } +} + +#[derive(Copy, Clone)] +struct VTable { + #[cfg(feature = "nightly")] + vtable: DynMetadata, + + #[cfg(not(feature = "nightly"))] + fat_ptr: NonNull, +} + +#[cfg(feature = "weak-ptrs")] +pub(crate) struct BoxedMetadata { + vtable: VTable, + pub(crate) weak_counter_marker: WeakCounterMarker, +} + +#[cfg(feature = "weak-ptrs")] +impl BoxedMetadata { + #[inline] + fn new(vtable: VTable, weak_counter_marker: WeakCounterMarker) -> NonNull { + unsafe { + let ptr: NonNull = alloc_other(); + ptr::write( + ptr.as_ptr(), + BoxedMetadata { + vtable, + weak_counter_marker, + }, + ); + ptr + } + } +} + // Trait used to make it possible to drop/finalize only the elem field of CcBox // and without taking a &mut reference to the whole CcBox trait InternalTrace: Trace { @@ -670,7 +799,7 @@ trait InternalTrace: Trace { unsafe fn drop_elem(&self); } -impl InternalTrace for CcBox { +impl InternalTrace for CcBox { #[cfg(feature = "finalization")] fn finalize_elem(&self) { self.get_elem().finalize(); @@ -680,3 +809,127 @@ impl InternalTrace for CcBox { drop_in_place(self.get_elem_mut()); } } + +// #################################### +// # Cc Trait impls # +// #################################### + +impl Default for Cc { + /// Creates a new [`Cc`][`Cc`], with the [`Default`] value for `T`. + /// + /// # Collection + /// + /// This method may start a collection when the `auto-collect` feature is enabled. + /// + /// See the [`config` module documentation][`mod@crate::config`] for more details. + #[inline] + fn default() -> Self { + Cc::new(::default()) + } +} + +impl AsRef for Cc { + #[inline(always)] + fn as_ref(&self) -> &T { + self + } +} + +impl Borrow for Cc { + #[inline(always)] + fn borrow(&self) -> &T { + self + } +} + +impl From for Cc { + /// Converts a generic `T` into a [`Cc`][`Cc`]. + /// + /// # Collection + /// + /// This method may start a collection when the `auto-collect` feature is enabled. + /// + /// See the [`config` module documentation][`mod@crate::config`] for more details. + #[inline] + fn from(value: T) -> Self { + Cc::new(value) + } +} + +// TODO impl From> for Cc +// TODO impl TryFrom for Cc when Cc::try_new will be implemented + +impl Debug for Cc { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Debug::fmt(&**self, f) + } +} + +impl Display for Cc { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&**self, f) + } +} + +impl Pointer for Cc { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Pointer::fmt(&ptr::addr_of!(**self), f) + } +} + +impl PartialEq for Cc { + #[inline] + fn eq(&self, other: &Self) -> bool { + **self == **other + } +} + +impl Eq for Cc {} + +impl Ord for Cc { + #[inline] + fn cmp(&self, other: &Self) -> Ordering { + (**self).cmp(&**other) + } +} + +impl PartialOrd for Cc { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + (**self).partial_cmp(&**other) + } + + #[inline] + fn lt(&self, other: &Self) -> bool { + **self < **other + } + + #[inline] + fn le(&self, other: &Self) -> bool { + **self <= **other + } + + #[inline] + fn gt(&self, other: &Self) -> bool { + **self > **other + } + + #[inline] + fn ge(&self, other: &Self) -> bool { + **self >= **other + } +} + +impl Hash for Cc { + #[inline] + fn hash(&self, state: &mut H) { + (**self).hash(state); + } +} + +impl UnwindSafe for Cc {} + +impl RefUnwindSafe for Cc {} diff --git a/src/cleaners/mod.rs b/src/cleaners/mod.rs index 06c4e15..dc397f5 100644 --- a/src/cleaners/mod.rs +++ b/src/cleaners/mod.rs @@ -16,7 +16,7 @@ //! //! # Avoiding memory leaks //! -//! Usually, [`Cleaner`]s are stored inside a cycle-collected object. Make sure to **never capture** a reference to the containing object +//! Usually, [`Cleaner`]s are stored inside a cycle-collected object. Make sure to **never capture** a reference to the container object //! inside the cleaning action closure, otherwise the object will be leaked and the cleaning action will never be executed. //! //! # Cleaners vs finalization @@ -26,18 +26,18 @@ use alloc::boxed::Box; use core::cell::RefCell; - +use core::fmt::{self, Debug, Formatter}; use slotmap::{new_key_type, SlotMap}; -use crate::{Context, Finalize, Trace}; -use crate::weak::{Weak, WeakableCc}; +use crate::{Cc, Context, Finalize, Trace}; +use crate::weak::Weak; new_key_type! { struct CleanerKey; } struct CleanerMap { - map: RefCell>>, // The Option is used to avoid allocating until a cleaning action is registered + map: RefCell>>, // The Option is used to avoid allocating until a cleaning action is registered } unsafe impl Trace for CleanerMap { @@ -48,9 +48,10 @@ unsafe impl Trace for CleanerMap { impl Finalize for CleanerMap {} -struct CleanerFn(Option>); +struct CleaningAction(Option>); -impl Drop for CleanerFn { +impl Drop for CleaningAction { + #[inline] fn drop(&mut self) { if let Some(fun) = self.0.take() { fun(); @@ -62,16 +63,15 @@ impl Drop for CleanerFn { /// /// All the cleaning actions registered in a `Cleaner` are run when it is dropped, unless they have been manually executed before. pub struct Cleaner { - cleaner_map: WeakableCc, + cleaner_map: Cc, } impl Cleaner { /// Creates a new [`Cleaner`]. - #[allow(clippy::new_without_default)] #[inline] pub fn new() -> Cleaner { Cleaner { - cleaner_map: WeakableCc::new_weakable(CleanerMap { + cleaner_map: Cc::new(CleanerMap { map: RefCell::new(None), }), } @@ -83,7 +83,7 @@ impl Cleaner { /// /// # Avoiding memory leaks /// Usually, [`Cleaner`]s are stored inside a cycle-collected object. Make sure to **never capture** - /// a reference to the containing object inside the `action` closure, otherwise the object will + /// a reference to the container object inside the `action` closure, otherwise the object will /// be leaked and the cleaning action will never be executed. #[inline] pub fn register(&self, action: impl FnOnce() + 'static) -> Cleanable { @@ -91,7 +91,7 @@ impl Cleaner { .map .borrow_mut() .get_or_insert_with(|| SlotMap::with_capacity_and_key(3)) - .insert(CleanerFn(Some(Box::new(action)))); + .insert(CleaningAction(Some(Box::new(action)))); Cleanable { cleaner_map: self.cleaner_map.downgrade(), @@ -110,13 +110,27 @@ unsafe impl Trace for Cleaner { fn trace(&self, _: &mut Context<'_>) { // DO NOT TRACE self.cleaner_map, it would be unsound! // If self.cleaner_map would be traced here, it would be possible to have cleaning actions called - // with a reference to the cleaned object accessible from inside the clean function. + // with a reference to the cleaned object accessible from inside the cleaning action itself. // This would be unsound, since cleaning actions are called from the Drop implementation of Ccs (see the Trace trait safety section) } } impl Finalize for Cleaner {} +impl Default for Cleaner { + #[inline] + fn default() -> Self { + Cleaner::new() + } +} + +impl Debug for Cleaner { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("Cleaner").finish_non_exhaustive() + } +} + /// A `Cleanable` represents a cleaning action registered in a [`Cleaner`]. pub struct Cleanable { cleaner_map: Weak, @@ -147,3 +161,10 @@ unsafe impl Trace for Cleanable { } impl Finalize for Cleanable {} + +impl Debug for Cleanable { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("Cleanable").finish_non_exhaustive() + } +} diff --git a/src/counter_marker.rs b/src/counter_marker.rs index acf67c7..cfe90ac 100644 --- a/src/counter_marker.rs +++ b/src/counter_marker.rs @@ -5,12 +5,14 @@ use crate::utils; const NON_MARKED: u32 = 0u32; const IN_POSSIBLE_CYCLES: u32 = 1u32 << (u32::BITS - 2); const TRACED: u32 = 2u32 << (u32::BITS - 2); +#[allow(dead_code)] // Used only in weak ptrs, silence warnings +const DROPPED: u32 = 3u32 << (u32::BITS - 2); const COUNTER_MASK: u32 = 0b11111111111111u32; // First 14 bits set to 1 const TRACING_COUNTER_MASK: u32 = COUNTER_MASK << 14; // 14 bits set to 1 followed by 14 bits set to 0 const FINALIZED_MASK: u32 = 1u32 << (u32::BITS - 4); -const DROPPED_MASK: u32 = 1u32 << (u32::BITS - 3); -const BITS_MASK: u32 = !(COUNTER_MASK | TRACING_COUNTER_MASK | FINALIZED_MASK | DROPPED_MASK); +const METADATA_MASK: u32 = 1u32 << (u32::BITS - 3); +const BITS_MASK: u32 = !(COUNTER_MASK | TRACING_COUNTER_MASK | FINALIZED_MASK | METADATA_MASK); const FIRST_BIT_MASK: u32 = 1u32 << (u32::BITS - 1); const INITIAL_VALUE: u32 = COUNTER_MASK + 2; // +2 means that tracing counter and counter are both set to 1 @@ -26,11 +28,12 @@ pub(crate) const MAX: u32 = COUNTER_MASK; /// +-----------+----------+----------+------------+------------+ /// ``` /// -/// * `A` has 3 possible states: +/// * `A` has 4 possible states: /// * `NON_MARKED` -/// * `IN_POSSIBLE_CYCLES` (this implies `NON_MARKED`) -/// * `TRACED` -/// * `B` is `1` when the element inside `CcBox` has already been dropped, `0` otherwise +/// * `IN_POSSIBLE_CYCLES`: in `possible_cycles` list (implies `NON_MARKED`) +/// * `TRACED`: in `root_list` or `non_root_list` +/// * `DROPPED`: allocated value has already been dropped (but not yet deallocated) +/// * `B` is `1` when metadata has been allocated, `0` otherwise /// * `C` is `1` when the element inside `CcBox` has already been finalized, `0` otherwise /// * `D` is the tracing counter /// * `E` is the counter (last one for sum/subtraction efficiency) @@ -133,19 +136,19 @@ impl CounterMarker { self.set_bit(finalized, FINALIZED_MASK); } - #[cfg(feature = "weak-ptr")] + #[cfg(feature = "weak-ptrs")] #[inline] - pub(crate) fn is_dropped(&self) -> bool { - (self.counter.get() & DROPPED_MASK) == DROPPED_MASK + pub(crate) fn has_allocated_for_metadata(&self) -> bool { + (self.counter.get() & METADATA_MASK) == METADATA_MASK } - #[cfg(feature = "weak-ptr")] + #[cfg(feature = "weak-ptrs")] #[inline] - pub(crate) fn set_dropped(&self, dropped: bool) { - self.set_bit(dropped, DROPPED_MASK); + pub(crate) fn set_allocated_for_metadata(&self, allocated_for_metadata: bool) { + self.set_bit(allocated_for_metadata, METADATA_MASK); } - #[cfg(any(feature = "weak-ptr", feature = "finalization"))] + #[cfg(any(feature = "weak-ptrs", feature = "finalization"))] #[inline(always)] fn set_bit(&self, value: bool, mask: u32) { if value { @@ -167,6 +170,19 @@ impl CounterMarker { (self.counter.get() & BITS_MASK) == TRACED } + #[inline] + pub(crate) fn is_traced_or_dropped(&self) -> bool { + // true if (self.counter & BITS_MASK) is equal to 10 or 11, + // so if the first bit is 1 + (self.counter.get() & FIRST_BIT_MASK) == FIRST_BIT_MASK + } + + #[cfg(any(feature = "weak-ptrs", all(test, feature = "std")))] + #[inline] + pub(crate) fn is_dropped(&self) -> bool { + (self.counter.get() & BITS_MASK) == DROPPED + } + #[inline] pub(crate) fn mark(&self, new_mark: Mark) { self.counter.set((self.counter.get() & !BITS_MASK) | (new_mark as u32)); @@ -179,4 +195,6 @@ pub(crate) enum Mark { NonMarked = NON_MARKED, PossibleCycles = IN_POSSIBLE_CYCLES, Traced = TRACED, + #[allow(dead_code)] // Used only in weak ptrs, silence warnings + Dropped = DROPPED, } diff --git a/src/lib.rs b/src/lib.rs index fc8f286..67221c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,28 +64,26 @@ collect_cycles(); //! ### Weak pointers //! #![cfg_attr( - feature = "weak-ptr", + feature = "weak-ptrs", doc = r"```rust" )] #![cfg_attr( - not(feature = "weak-ptr"), + not(feature = "weak-ptrs"), doc = r"```rust,ignore" )] #![doc = r"# use rust_cc::*; # use rust_cc::weak::*; -// Only Ccs containing a Weakable<_> support weak pointers -// (WeakableCc is just an alias for Cc>) -let weakable: WeakableCc = Cc::new_weakable(5); +let cc: Cc = Cc::new(5); // Obtain a weak pointer -let weak_ptr: Weak = weakable.downgrade(); +let weak_ptr: Weak = cc.downgrade(); // Upgrading a weak pointer cannot fail if the pointed allocation isn't deallocated -let upgraded: Option> = weak_ptr.upgrade(); +let upgraded: Option> = weak_ptr.upgrade(); assert!(upgraded.is_some()); // Deallocate the object -drop(weakable); +drop(cc); drop(upgraded); // Upgrading now fails @@ -140,7 +138,6 @@ cleanable.clean(); #![deny(rustdoc::broken_intra_doc_links)] #![allow(clippy::thread_local_initializer_can_be_made_const)] -#![allow(unexpected_cfgs)] #[cfg(all(not(feature = "std"), not(feature = "nightly")))] compile_error!("Feature \"std\" cannot be disabled without enabling feature \"nightly\" (due to #[thread_local] not being stable)."); @@ -176,7 +173,7 @@ pub mod config; #[cfg(feature = "derive")] mod derives; -#[cfg(feature = "weak-ptr")] +#[cfg(feature = "weak-ptrs")] pub mod weak; #[cfg(feature = "cleaners")] @@ -212,19 +209,17 @@ pub fn collect_cycles() { #[cfg(feature = "auto-collect")] #[inline(never)] -pub(crate) fn trigger_collection() { - let _ = try_state(|state| { - if state.is_collecting() { - return; - } +pub(crate) fn trigger_collection(state: &State) { + if state.is_collecting() { + return; + } - let _ = POSSIBLE_CYCLES.try_with(|pc| { - if config::config(|config| config.should_collect(state, pc)).unwrap_or(false) { - collect(state, pc); + let _ = POSSIBLE_CYCLES.try_with(|pc| { + if config::config(|config| config.should_collect(state, pc)).unwrap_or(false) { + collect(state, pc); - adjust_trigger_point(state); - } - }); + adjust_trigger_point(state); + } }); } @@ -397,15 +392,16 @@ fn deallocate_list(to_deallocate_list: List, state: &State) { impl Drop for ToDropList { #[inline] fn drop(&mut self) { - // Remove the remaining elements from the list, setting them as dropped + // Remove the elements from the list, setting them as dropped // This feature is used only in weak pointers, so do this only if they're enabled - #[cfg(feature = "weak-ptr")] + #[cfg(feature = "weak-ptrs")] while let Some(ptr) = self.list.remove_first() { - unsafe { ptr.as_ref() }.counter_marker().set_dropped(true); + // Always set the mark, since it has been cleared by remove_first + unsafe { ptr.as_ref() }.counter_marker().mark(Mark::Dropped); } // If not using weak pointers, just call the list's drop implementation - #[cfg(not(feature = "weak-ptr"))] + #[cfg(not(feature = "weak-ptrs"))] unsafe { ManuallyDrop::drop(&mut self.list); } @@ -425,12 +421,8 @@ fn deallocate_list(to_deallocate_list: List, state: &State) { unsafe { debug_assert!(ptr.as_ref().counter_marker().is_traced()); - // Set the object as dropped before dropping it - // This feature is used only in weak pointers, so do this only if they're enabled - #[cfg(feature = "weak-ptr")] - { - ptr.as_ref().counter_marker().set_dropped(true); - } + #[cfg(feature = "weak-ptrs")] + ptr.as_ref().drop_metadata(); CcBox::drop_inner(ptr.cast()); }; diff --git a/src/tests/counter_marker.rs b/src/tests/counter_marker.rs index 2371ea4..e1a6bd5 100644 --- a/src/tests/counter_marker.rs +++ b/src/tests/counter_marker.rs @@ -1,11 +1,16 @@ use crate::counter_marker::*; +fn assert_not_marked(counter: &CounterMarker) { + assert!(counter.is_not_marked()); + assert!(!counter.is_in_possible_cycles()); + assert!(!counter.is_traced()); + assert!(!counter.is_dropped()); +} + #[test] fn test_new() { fn test(counter: CounterMarker) { - assert!(counter.is_not_marked()); - assert!(!counter.is_in_possible_cycles()); - assert!(!counter.is_traced()); + assert_not_marked(&counter); assert_eq!(counter.counter(), 1); assert_eq!(counter.tracing_counter(), 1); @@ -19,63 +24,69 @@ fn test_new() { #[test] fn test_is_to_finalize() { let counter = CounterMarker::new_with_counter_to_one(false); + assert_not_marked(&counter); assert!(counter.needs_finalization()); let counter = CounterMarker::new_with_counter_to_one(false); + assert_not_marked(&counter); counter.set_finalized(true); assert!(!counter.needs_finalization()); let counter = CounterMarker::new_with_counter_to_one(false); + assert_not_marked(&counter); counter.set_finalized(false); assert!(counter.needs_finalization()); let counter = CounterMarker::new_with_counter_to_one(true); + assert_not_marked(&counter); assert!(!counter.needs_finalization()); let counter = CounterMarker::new_with_counter_to_one(true); + assert_not_marked(&counter); counter.set_finalized(true); assert!(!counter.needs_finalization()); let counter = CounterMarker::new_with_counter_to_one(true); + assert_not_marked(&counter); counter.set_finalized(false); assert!(counter.needs_finalization()); } -#[cfg(feature = "weak-ptr")] +#[cfg(feature = "weak-ptrs")] #[test] -fn test_is_dropped() { +fn test_weak_ptrs_exists() { let counter = CounterMarker::new_with_counter_to_one(false); - assert!(!counter.is_dropped()); + assert_not_marked(&counter); + assert!(!counter.has_allocated_for_metadata()); let counter = CounterMarker::new_with_counter_to_one(false); - counter.set_dropped(true); - assert!(counter.is_dropped()); + assert_not_marked(&counter); + counter.set_allocated_for_metadata(true); + assert!(counter.has_allocated_for_metadata()); let counter = CounterMarker::new_with_counter_to_one(false); - counter.set_dropped(false); - assert!(!counter.is_dropped()); + assert_not_marked(&counter); + counter.set_allocated_for_metadata(false); + assert!(!counter.has_allocated_for_metadata()); let counter = CounterMarker::new_with_counter_to_one(true); - assert!(!counter.is_dropped()); + assert_not_marked(&counter); + assert!(!counter.has_allocated_for_metadata()); let counter = CounterMarker::new_with_counter_to_one(true); - counter.set_dropped(true); - assert!(counter.is_dropped()); + assert_not_marked(&counter); + counter.set_allocated_for_metadata(true); + assert!(counter.has_allocated_for_metadata()); let counter = CounterMarker::new_with_counter_to_one(true); - counter.set_dropped(false); - assert!(!counter.is_dropped()); + assert_not_marked(&counter); + counter.set_allocated_for_metadata(false); + assert!(!counter.has_allocated_for_metadata()); } #[test] fn test_increment_decrement() { fn test(counter: CounterMarker) { - fn assert_not_marked(counter: &CounterMarker) { - assert!(counter.is_not_marked()); - assert!(!counter.is_in_possible_cycles()); - assert!(!counter.is_traced()); - } - assert_not_marked(&counter); assert_eq!(counter.counter(), 1); @@ -147,33 +158,36 @@ fn test_increment_decrement() { #[test] fn test_marks() { fn test(counter: CounterMarker) { - assert!(counter.is_not_marked()); - assert!(!counter.is_in_possible_cycles()); - assert!(!counter.is_traced()); + assert_not_marked(&counter); counter.mark(Mark::NonMarked); - assert!(counter.is_not_marked()); - assert!(!counter.is_in_possible_cycles()); - assert!(!counter.is_traced()); + assert_not_marked(&counter); counter.mark(Mark::PossibleCycles); assert!(counter.is_not_marked()); assert!(counter.is_in_possible_cycles()); assert!(!counter.is_traced()); + assert!(!counter.is_dropped()); counter.mark(Mark::Traced); assert!(!counter.is_not_marked()); assert!(!counter.is_in_possible_cycles()); assert!(counter.is_traced()); + assert!(!counter.is_dropped()); - counter.mark(Mark::NonMarked); + counter.mark(Mark::Dropped); - assert!(counter.is_not_marked()); + assert!(!counter.is_not_marked()); assert!(!counter.is_in_possible_cycles()); assert!(!counter.is_traced()); + assert!(counter.is_dropped()); + + counter.mark(Mark::NonMarked); + + assert_not_marked(&counter); } test(CounterMarker::new_with_counter_to_one(false)); diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 1c44185..f7d4f4c 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -15,7 +15,7 @@ mod list; mod panicking; mod counter_marker; -#[cfg(feature = "weak-ptr")] +#[cfg(feature = "weak-ptrs")] mod weak; #[cfg(feature = "cleaners")] diff --git a/src/tests/weak/mod.rs b/src/tests/weak/mod.rs index 07fc350..14ac1be 100644 --- a/src/tests/weak/mod.rs +++ b/src/tests/weak/mod.rs @@ -1,9 +1,8 @@ use std::cell::Cell; -use std::ops::Deref; use std::panic::catch_unwind; use crate::*; use super::reset_state; -use crate::weak::{Weak, Weakable, WeakableCc}; +use crate::weak::Weak; #[test] fn weak_test() { @@ -44,22 +43,22 @@ fn weak_test2() { collect_cycles(); } -fn weak_test_common() -> (WeakableCc, Weak) { +fn weak_test_common() -> (Cc, Weak) { reset_state(); - let cc: Cc> = WeakableCc::new_weakable(0i32); + let cc: Cc = Cc::new(0i32); - assert!(!cc.deref().has_allocated()); + assert!(!cc.inner().counter_marker().has_allocated_for_metadata()); assert_eq!(0, cc.weak_count()); assert_eq!(1, cc.strong_count()); let cc1 = cc.clone(); - assert!(!cc.deref().has_allocated()); + assert!(!cc.inner().counter_marker().has_allocated_for_metadata()); let weak = cc.downgrade(); - assert!(cc.deref().has_allocated()); + assert!(cc.inner().counter_marker().has_allocated_for_metadata()); assert_eq!(2, cc.strong_count()); assert_eq!(1, weak.weak_count()); @@ -103,8 +102,8 @@ fn weak_test_common() -> (WeakableCc, Weak) { fn weak_dst() { reset_state(); - let cc = Cc::new_weakable(0i32); - let cc1: WeakableCc = cc.clone(); + let cc = Cc::new(0i32); + let cc1: Cc = cc.clone(); let _weak: Weak = cc.downgrade(); let _weak1: Weak = cc1.downgrade(); } @@ -136,7 +135,7 @@ fn test_new_cyclic() { } }); - assert!(cyclic.deref().has_allocated()); + assert!(cyclic.inner().counter_marker().has_allocated_for_metadata()); assert_eq!(1, cyclic.weak_count()); assert_eq!(1, cyclic.strong_count()); @@ -150,7 +149,8 @@ fn test_new_cyclic() { fn panicking_new_cyclic1() { reset_state(); - let _cc = Cc::new_cyclic(|_| { + #[allow(clippy::unused_unit)] // Unit type needed to fix never type fallback error + let _cc = Cc::new_cyclic(|_| -> () { panic!("Expected panic during panicking_new_cyclic1!"); }); } @@ -160,7 +160,8 @@ fn panicking_new_cyclic1() { fn panicking_new_cyclic2() { reset_state(); - let _cc = Cc::new_cyclic(|weak| { + #[allow(clippy::unused_unit)] // Unit type needed to fix never type fallback error + let _cc = Cc::new_cyclic(|weak| -> () { let _weak = weak.clone(); panic!("Expected panic during panicking_new_cyclic2!"); }); @@ -270,7 +271,7 @@ fn try_upgrade_and_resurrect_in_finalize_and_drop() { reset_state(); thread_local! { - static RESURRECTED: Cell>> = Cell::new(None); + static RESURRECTED: Cell>> = Cell::new(None); } struct Cyclic { @@ -322,7 +323,7 @@ fn try_upgrade_in_cyclic_finalize_and_drop() { } struct Cyclic { - cyclic: RefCell>>, + cyclic: RefCell>>, weak: Weak, } @@ -349,7 +350,7 @@ fn try_upgrade_in_cyclic_finalize_and_drop() { } let weak: Weak = { - let cc: Cc> = WeakableCc::new_cyclic(|weak| Cyclic { + let cc: Cc = Cc::new_cyclic(|weak| Cyclic { cyclic: RefCell::new(None), weak: weak.clone(), }); @@ -373,3 +374,41 @@ fn try_upgrade_in_cyclic_finalize_and_drop() { } assert!(DROPPED.with(|dropped| dropped.get())); } + +#[test] +fn weak_new() { + reset_state(); + + let new: Weak = Weak::new(); + + assert_eq!(0, new.weak_count()); + assert_eq!(0, new.strong_count()); + assert!(new.upgrade().is_none()); + + let other = new.clone(); + + assert_eq!(0, other.weak_count()); + assert_eq!(0, other.strong_count()); + assert!(other.upgrade().is_none()); +} + +#[test] +fn weak_new_ptr_eq() { + reset_state(); + + let new: Weak = Weak::new(); + let other_new: Weak = Weak::new(); + + let cc = Cc::new(5u32); + let other_cc = Cc::new(6u32); + + assert_eq!(0, new.weak_count()); + assert_eq!(0, new.strong_count()); + + assert!(Weak::ptr_eq(&new, &new)); + assert!(Weak::ptr_eq(&new, &other_new)); + assert!(Weak::ptr_eq(&new, &new.clone())); + assert!(!Weak::ptr_eq(&new, &cc.downgrade())); + assert!(Weak::ptr_eq(&cc.downgrade(), &cc.downgrade())); + assert!(!Weak::ptr_eq(&cc.downgrade(), &other_cc.downgrade())); +} diff --git a/src/trace.rs b/src/trace.rs index 0208283..34a55dd 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -253,16 +253,16 @@ impl Finalize for MaybeUninit { fn finalize(&self) {} }*/ -unsafe impl Trace for PhantomData { +unsafe impl Trace for PhantomData { #[inline(always)] fn trace(&self, _: &mut Context<'_>) {} } -impl Finalize for PhantomData {} +impl Finalize for PhantomData {} macro_rules! deref_trace { ($generic:ident; $this:ty; $($bound:tt)*) => { - unsafe impl<$generic: $($bound)* $crate::trace::Trace + 'static> $crate::trace::Trace for $this + unsafe impl<$generic: $($bound)* $crate::trace::Trace> $crate::trace::Trace for $this { #[inline] fn trace(&self, ctx: &mut $crate::trace::Context<'_>) { @@ -271,7 +271,7 @@ macro_rules! deref_trace { } } - impl<$generic: $($bound)* $crate::trace::Finalize + 'static> $crate::trace::Finalize for $this + impl<$generic: $($bound)* $crate::trace::Finalize> $crate::trace::Finalize for $this { #[inline] fn finalize(&self) { @@ -307,16 +307,16 @@ deref_traces_sized! { AssertUnwindSafe, } -unsafe impl Trace for RefCell { +unsafe impl Trace for RefCell { #[inline] fn trace(&self, ctx: &mut Context<'_>) { - if let Ok(borrow) = self.try_borrow() { + if let Ok(borrow) = self.try_borrow_mut() { borrow.trace(ctx); } } } -impl Finalize for RefCell { +impl Finalize for RefCell { #[inline] fn finalize(&self) { if let Ok(borrow) = self.try_borrow() { @@ -325,7 +325,7 @@ impl Finalize for RefCell { } } -unsafe impl Trace for Option { +unsafe impl Trace for Option { #[inline] fn trace(&self, ctx: &mut Context<'_>) { if let Some(inner) = self { @@ -334,7 +334,7 @@ unsafe impl Trace for Option { } } -impl Finalize for Option { +impl Finalize for Option { #[inline] fn finalize(&self) { if let Some(value) = self { @@ -343,7 +343,7 @@ impl Finalize for Option { } } -unsafe impl Trace for Result { +unsafe impl Trace for Result { #[inline] fn trace(&self, ctx: &mut Context<'_>) { match self { @@ -353,7 +353,7 @@ unsafe impl Trace for Result { } } -impl Finalize for Result { +impl Finalize for Result { #[inline] fn finalize(&self) { match self { @@ -363,7 +363,7 @@ impl Finalize for Result { } } -unsafe impl Trace for [T; N] { +unsafe impl Trace for [T; N] { #[inline] fn trace(&self, ctx: &mut Context<'_>) { for elem in self { @@ -372,7 +372,7 @@ unsafe impl Trace for [T; N] { } } -impl Finalize for [T; N] { +impl Finalize for [T; N] { #[inline] fn finalize(&self) { for elem in self { @@ -381,7 +381,7 @@ impl Finalize for [T; N] { } } -unsafe impl Trace for [T] { +unsafe impl Trace for [T] { #[inline] fn trace(&self, ctx: &mut Context<'_>) { for elem in self { @@ -390,7 +390,7 @@ unsafe impl Trace for [T] { } } -impl Finalize for [T] { +impl Finalize for [T] { #[inline] fn finalize(&self) { for elem in self { @@ -399,7 +399,7 @@ impl Finalize for [T] { } } -unsafe impl Trace for Vec { +unsafe impl Trace for Vec { #[inline] fn trace(&self, ctx: &mut Context<'_>) { for elem in self { @@ -408,7 +408,7 @@ unsafe impl Trace for Vec { } } -impl Finalize for Vec { +impl Finalize for Vec { #[inline] fn finalize(&self) { for elem in self { @@ -421,7 +421,7 @@ macro_rules! tuple_finalize_trace { ($($args:ident),+) => { #[allow(non_snake_case)] unsafe impl<$($args),*> $crate::trace::Trace for ($($args,)*) - where $($args: $crate::trace::Trace + 'static),* + where $($args: $crate::trace::Trace),* { #[inline] fn trace(&self, ctx: &mut $crate::trace::Context<'_>) { @@ -437,7 +437,7 @@ macro_rules! tuple_finalize_trace { #[allow(non_snake_case)] impl<$($args),*> $crate::trace::Finalize for ($($args,)*) - where $($args: $crate::trace::Finalize + 'static),* + where $($args: $crate::trace::Finalize),* { #[inline] fn finalize(&self) { diff --git a/src/utils.rs b/src/utils.rs index 405c2ef..5277eb5 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -23,7 +23,7 @@ pub(crate) unsafe fn cc_dealloc( dealloc(ptr.cast().as_ptr(), layout); } -#[cfg(any(feature = "weak-ptr", feature = "cleaners"))] +#[cfg(any(feature = "weak-ptrs", feature = "cleaners"))] #[inline] pub(crate) unsafe fn alloc_other() -> NonNull { let layout = Layout::new::(); @@ -33,7 +33,7 @@ pub(crate) unsafe fn alloc_other() -> NonNull { } } -#[cfg(any(feature = "weak-ptr", feature = "cleaners"))] +#[cfg(any(feature = "weak-ptrs", feature = "cleaners"))] #[inline] pub(crate) unsafe fn dealloc_other(ptr: NonNull) { let layout = Layout::new::(); @@ -95,7 +95,7 @@ mod no_std_thread_locals { value: T, } - impl NoStdLocalKey { + impl NoStdLocalKey { #[inline(always)] pub(crate) const fn new(value: T) -> Self { NoStdLocalKey { value } diff --git a/src/weak/mod.rs b/src/weak/mod.rs index 3a2518f..975bc38 100644 --- a/src/weak/mod.rs +++ b/src/weak/mod.rs @@ -1,11 +1,10 @@ //! Non-owning [`Weak`] pointers to an allocation. //! -//! The [`downgrade`][`method@Cc::downgrade`] method can be used on a [`WeakableCc`] to create a non-owning [`Weak`][`crate::weak::Weak`] pointer. +//! The [`downgrade`][`method@Cc::downgrade`] method can be used on a [`Cc`] to create a non-owning [`Weak`][`crate::weak::Weak`] pointer. //! A [`Weak`][`crate::weak::Weak`] pointer can be [`upgrade`][`method@Weak::upgrade`]d to a [`Cc`], but this will return //! [`None`] if the allocation has already been deallocated. use alloc::rc::Rc; -use core::ops::Deref; use core::{mem, ptr}; use core::ptr::{drop_in_place, NonNull}; #[cfg(feature = "nightly")] @@ -13,25 +12,22 @@ use core::{ marker::Unsize, ops::CoerceUnsized, }; -use core::cell::Cell; +use core::fmt::{self, Debug, Formatter}; use core::mem::MaybeUninit; use core::marker::PhantomData; -use crate::cc::CcBox; +use crate::cc::{BoxedMetadata, CcBox}; use crate::state::try_state; use crate::{Cc, Context, Finalize, Trace}; -use crate::utils::{alloc_other, cc_dealloc, dealloc_other}; -use crate::weak::weak_metadata::WeakMetadata; +use crate::utils::{cc_dealloc, dealloc_other}; +use crate::weak::weak_counter_marker::WeakCounterMarker; -mod weak_metadata; - -/// A [`Cc`] which can be [`downgrade`][`method@Cc::downgrade`]d to a [`Weak`] pointer. -pub type WeakableCc = Cc>; +pub(crate) mod weak_counter_marker; /// A non-owning pointer to the managed allocation. pub struct Weak { - metadata: NonNull, - cc: NonNull>>, + metadata: Option>, // None when created using Weak::new() + cc: NonNull>, _phantom: PhantomData>, // Make Weak !Send and !Sync } @@ -43,7 +39,19 @@ impl CoerceUnsized> for Weak { } -impl Weak { +impl Weak { + /// Constructs a new [`Weak`][`Weak`], without allocating any memory. Calling [`upgrade`][`method@Weak::upgrade`] on the returned value always gives [`None`]. + #[inline] + pub fn new() -> Self { + Weak { + metadata: None, + cc: NonNull::dangling(), + _phantom: PhantomData, + } + } +} + +impl Weak { /// Tries to upgrade the weak pointer to a [`Cc`], returning [`None`] if the allocation has already been deallocated. /// /// This creates a [`Cc`] pointer to the managed allocation, increasing the strong reference count. @@ -54,7 +62,7 @@ impl Weak { #[inline] #[must_use = "newly created Cc is immediately dropped"] #[track_caller] - pub fn upgrade(&self) -> Option>> { + pub fn upgrade(&self) -> Option> { #[cfg(debug_assertions)] if crate::state::state(|state| state.is_tracing()) { panic!("Cannot upgrade while tracing!"); @@ -74,16 +82,25 @@ impl Weak { } } - /// Returns `true` if the two [`Weak`]s point to the same allocation. This function ignores the metadata of `dyn Trait` pointers. + /// Returns `true` if the two [`Weak`]s point to the same allocation, or if both don’t point to any allocation + /// (because they were created with [`Weak::new()`][`Weak::new`]). This function ignores the metadata of `dyn Trait` pointers. #[inline] pub fn ptr_eq(this: &Weak, other: &Weak) -> bool { - ptr::eq(this.metadata.as_ptr() as *const (), other.metadata.as_ptr() as *const ()) + match (this.metadata, other.metadata) { + (None, None) => true, + (None, Some(_)) => false, + (Some(_), None) => false, + // Only compare the metadata allocations since they're surely unique + (Some(m1), Some(m2)) => ptr::eq(m1.as_ptr() as *const (), m2.as_ptr() as *const ()), + } } /// Returns the number of [`Cc`]s to the pointed allocation. + /// + /// If `self` was created using [`Weak::new`], this will return 0. #[inline] pub fn strong_count(&self) -> u32 { - if self.metadata().is_accessible() { + if self.weak_counter_marker().map_or(false, |wcm| wcm.is_accessible()) { // SAFETY: self.cc is still allocated and can be dereferenced let counter_marker = unsafe { self.cc.as_ref() }.counter_marker(); @@ -97,7 +114,9 @@ impl Weak { let counter = counter_marker.counter(); // Checking if the counter is already 0 avoids doing extra useless work, since the returned value would be the same - if counter == 0 || counter_marker.is_dropped() || (counter_marker.is_traced() && try_state(|state| state.is_dropping()).unwrap_or(true)) { + if counter == 0 || counter_marker.is_dropped() || ( + counter_marker.is_traced() && try_state(|state| state.is_dropping()).unwrap_or(true) + ) { 0 } else { counter @@ -108,19 +127,21 @@ impl Weak { } /// Returns the number of [`Weak`]s to the pointed allocation. + /// + /// If `self` was created using [`Weak::new`], this will return 0. #[inline] pub fn weak_count(&self) -> u32 { // This function returns an u32 although internally the weak counter is an u16 to have more flexibility for future expansions - self.metadata().counter() as u32 + self.weak_counter_marker().map_or(0, |wcm| wcm.counter() as u32) } - #[inline(always)] - fn metadata(&self) -> &WeakMetadata { - unsafe { self.metadata.as_ref() } + #[inline] + fn weak_counter_marker(&self) -> Option<&WeakCounterMarker> { + Some(unsafe { &self.metadata?.as_ref().weak_counter_marker }) } } -impl Clone for Weak { +impl Clone for Weak { /// Makes a clone of the [`Weak`] pointer. /// /// This creates another [`Weak`] pointer to the same allocation, increasing the weak reference count. @@ -136,8 +157,10 @@ impl Clone for Weak { panic!("Cannot clone while tracing!"); } - if self.metadata().increment_counter().is_err() { - panic!("Too many references has been created to a single Weak"); + if let Some(wcm) = self.weak_counter_marker() { + if wcm.increment_counter().is_err() { + panic!("Too many references has been created to a single Weak"); + } } Weak { @@ -148,90 +171,36 @@ impl Clone for Weak { } } -impl Drop for Weak { +impl Drop for Weak { #[inline] fn drop(&mut self) { - // Always decrement the weak counter - let res = self.metadata().decrement_counter(); - debug_assert!(res.is_ok()); - - if self.metadata().counter() == 0 && !self.metadata().is_accessible() { - // No weak pointer is left and the CcBox has been deallocated, so just deallocate the metadata - unsafe { - dealloc_other(self.metadata); + let Some(metadata) = self.metadata else { return; }; + + unsafe { + // Always decrement the weak counter + let res = metadata.as_ref().weak_counter_marker.decrement_counter(); + debug_assert!(res.is_ok()); + + if metadata.as_ref().weak_counter_marker.counter() == 0 && !metadata.as_ref().weak_counter_marker.is_accessible() { + // No weak pointer is left and the CcBox has been deallocated, so just deallocate the metadata + dealloc_other(metadata); } } } } -unsafe impl Trace for Weak { +unsafe impl Trace for Weak { #[inline(always)] fn trace(&self, _: &mut Context<'_>) { // Do not trace anything here, otherwise it wouldn't be a weak pointer } } -impl Finalize for Weak { -} - -/// Enables to [`downgrade`][`method@Cc::downgrade`] a [`Cc`] to a [`Weak`] pointer. -pub struct Weakable { - metadata: Cell>>, // the Option is used to avoid allocating until a Weak is created - _phantom: PhantomData>, // Make Weakable !Send and !Sync - elem: T, -} - -impl Weakable { - /// Creates a new `Weakable`. - #[inline(always)] - #[must_use = "newly created Weakable is immediately dropped"] - pub fn new(t: T) -> Weakable { - Weakable { - metadata: Cell::new(None), - _phantom: PhantomData, - elem: t, - } - } -} - -#[inline] -fn alloc_metadata(metadata: WeakMetadata) -> NonNull { - unsafe { - let ptr: NonNull = alloc_other(); - ptr::write( - ptr.as_ptr(), - metadata, - ); - ptr - } -} - -impl Weakable { - #[inline] - fn init_get_metadata(&self) -> NonNull { - self.metadata.get().unwrap_or_else(|| { - let ptr = alloc_metadata(WeakMetadata::new(true)); - self.metadata.set(Some(ptr)); - ptr - }) - } - - #[cfg(all(test, feature = "std"))] // Only used in unit tests - pub(crate) fn has_allocated(&self) -> bool { - self.metadata.get().is_some() - } +impl Finalize for Weak { } -impl Cc> { - /// Creates a new [`WeakableCc`]. - #[inline(always)] - #[must_use = "newly created Cc is immediately dropped"] - #[track_caller] - pub fn new_weakable(t: T) -> Self { - Cc::new(Weakable::new(t)) - } - - /// Creates a new [`WeakableCc`][`WeakableCc`] while providing a [`Weak`][`Weak`] pointer to the allocation, +impl Cc { + /// Creates a new [`Cc`][`Cc`] while providing a [`Weak`][`Weak`] pointer to the allocation, /// to allow the creation of a `T` which holds a weak pointer to itself. /// /// # Collection @@ -270,7 +239,7 @@ let cyclic = Cc::new_cyclic(|weak| { ```"] #[must_use = "newly created Cc is immediately dropped"] #[track_caller] - pub fn new_cyclic(f: F) -> Cc> + pub fn new_cyclic(f: F) -> Cc where F: FnOnce(&Weak) -> T, { @@ -279,7 +248,7 @@ let cyclic = Cc::new_cyclic(|weak| { panic!("Cannot create a new Cc while tracing!"); } - let cc = Cc::new(Weakable::new(NewCyclicWrapper::new())); + let cc = Cc::new(NewCyclicWrapper::new()); // Immediately call inner_ptr and forget the Cc instance. Having a Cc instance is dangerous, since: // 1. The strong count will become 0 @@ -287,11 +256,11 @@ let cyclic = Cc::new_cyclic(|weak| { let invalid_cc: NonNull> = cc.inner_ptr(); mem::forget(cc); - let metadata: NonNull = unsafe { invalid_cc.as_ref() }.get_elem().init_get_metadata(); + let metadata: NonNull = unsafe { invalid_cc.as_ref() }.get_or_init_metadata(); // Set weak counter to 1 // This is done after creating the Cc to make sure that if Cc::new panics the metadata allocation isn't leaked - let _ = unsafe { metadata.as_ref() }.increment_counter(); + let _ = unsafe { metadata.as_ref() }.weak_counter_marker.increment_counter(); { let counter_marker = unsafe { invalid_cc.as_ref() }.counter_marker(); @@ -304,21 +273,21 @@ let cyclic = Cc::new_cyclic(|weak| { } let weak: Weak = Weak { - metadata, + metadata: Some(metadata), cc: invalid_cc.cast(), // This cast is correct since NewCyclicWrapper is repr(transparent) and contains a MaybeUninit _phantom: PhantomData, }; // Panic guard to deallocate the metadata and the CcBox if the provided function f panics struct PanicGuard { - invalid_cc: NonNull>>>, + invalid_cc: NonNull>>, } - impl Drop for PanicGuard { + impl Drop for PanicGuard { fn drop(&mut self) { unsafe { // Deallocate only the metadata allocation - (*self.invalid_cc.as_ref().get_elem_mut()).drop_metadata(); + self.invalid_cc.as_ref().drop_metadata(); // Deallocate the CcBox. Use try_state to avoid panicking inside a Drop let _ = try_state(|state| { let layout = self.invalid_cc.as_ref().layout(); @@ -334,7 +303,7 @@ let cyclic = Cc::new_cyclic(|weak| { unsafe { // Write the newly created T - (*invalid_cc.as_ref().get_elem_mut()).elem.inner.write(to_write); + (*invalid_cc.as_ref().get_elem_mut()).inner.write(to_write); } // Set strong count to 1 @@ -343,7 +312,7 @@ let cyclic = Cc::new_cyclic(|weak| { // Create the Cc again since it is now valid // Casting invalid_cc is correct since NewCyclicWrapper is repr(transparent) and contains a MaybeUninit - let cc: Cc> = Cc::__new_internal(invalid_cc.cast()); + let cc: Cc = Cc::__new_internal(invalid_cc.cast()); debug_assert_eq!(1, cc.inner().counter_marker().counter()); @@ -353,7 +322,7 @@ let cyclic = Cc::new_cyclic(|weak| { } } -impl Cc> { +impl Cc { /// Creates a new [`Weak`] pointer to the managed allocation, increasing the weak reference count. /// /// # Panics @@ -368,16 +337,16 @@ impl Cc> { panic!("Cannot downgrade while tracing!"); } - let metadata = self.init_get_metadata(); + let metadata = self.inner().get_or_init_metadata(); - if unsafe { metadata.as_ref() }.increment_counter().is_err() { + if unsafe { metadata.as_ref() }.weak_counter_marker.increment_counter().is_err() { panic!("Too many references has been created to a single Weak"); } self.mark_alive(); Weak { - metadata, + metadata: Some(metadata), cc: self.inner_ptr(), _phantom: PhantomData, } @@ -387,68 +356,23 @@ impl Cc> { #[inline] pub fn weak_count(&self) -> u32 { // This function returns an u32 although internally the weak counter is an u16 to have more flexibility for future expansions - if let Some(metadata) = self.metadata.get() { - unsafe { metadata.as_ref().counter() as u32 } + if self.inner().counter_marker().has_allocated_for_metadata() { + // SAFETY: The metadata has been allocated + unsafe { self.inner().get_metadata_unchecked().as_ref() }.weak_counter_marker.counter() as u32 } else { 0 } } } -impl Weakable { - fn drop_metadata(&mut self) { - if let Some(metadata) = self.metadata.get() { - unsafe { - if metadata.as_ref().counter() == 0 { - // There are no weak pointers, deallocate the metadata - dealloc_other(metadata); - } else { - // There exist weak pointers, set the CcBox allocation not accessible - metadata.as_ref().set_accessible(false); - } - } - } - } -} - -impl Deref for Weakable { - type Target = T; - - #[inline] - fn deref(&self) -> &Self::Target { - &self.elem - } -} - -impl Drop for Weakable { - #[inline] - fn drop(&mut self) { - self.drop_metadata(); - } -} - -unsafe impl Trace for Weakable { - #[inline] - fn trace(&self, ctx: &mut Context<'_>) { - self.elem.trace(ctx); - } -} - -impl Finalize for Weakable { - #[inline] - fn finalize(&self) { - // Weakable is intended to be a transparent wrapper, so just call finalize on elem - self.elem.finalize(); - } -} - /// A **transparent** wrapper used to implement [`Cc::new_cyclic`]. #[repr(transparent)] struct NewCyclicWrapper { inner: MaybeUninit, } -impl NewCyclicWrapper { +impl NewCyclicWrapper { + #[inline(always)] fn new() -> NewCyclicWrapper { NewCyclicWrapper { inner: MaybeUninit::uninit(), @@ -456,7 +380,8 @@ impl NewCyclicWrapper { } } -unsafe impl Trace for NewCyclicWrapper { +unsafe impl Trace for NewCyclicWrapper { + #[inline] fn trace(&self, ctx: &mut Context<'_>) { // SAFETY: NewCyclicWrapper is used only in new_cyclic and a traceable Cc instance is not constructed until the contents are initialized unsafe { @@ -465,7 +390,8 @@ unsafe impl Trace for NewCyclicWrapper { } } -impl Finalize for NewCyclicWrapper { +impl Finalize for NewCyclicWrapper { + #[inline] fn finalize(&self) { // SAFETY: NewCyclicWrapper is used only in new_cyclic and a traceable Cc instance is not constructed until the contents are initialized unsafe { @@ -474,7 +400,8 @@ impl Finalize for NewCyclicWrapper { } } -impl Drop for NewCyclicWrapper { +impl Drop for NewCyclicWrapper { + #[inline] fn drop(&mut self) { // SAFETY: NewCyclicWrapper is used only in new_cyclic and a traceable Cc instance is not constructed until the contents are initialized unsafe { @@ -483,3 +410,21 @@ impl Drop for NewCyclicWrapper { } } } + +// #################################### +// # Weak Trait impls # +// #################################### + +impl Default for Weak { + #[inline] + fn default() -> Self { + Weak::new() + } +} + +impl Debug for Weak { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "(Weak)") + } +} diff --git a/src/weak/weak_metadata.rs b/src/weak/weak_counter_marker.rs similarity index 93% rename from src/weak/weak_metadata.rs rename to src/weak/weak_counter_marker.rs index 6a9994e..fa6bffa 100644 --- a/src/weak/weak_metadata.rs +++ b/src/weak/weak_counter_marker.rs @@ -19,17 +19,17 @@ pub(crate) const MAX: u16 = !ACCESSIBLE_MASK; // First 15 bits to 1 /// * `A` is `1` when the `Cc` is accessible, `0` otherwise /// * `B` is the weak counter #[derive(Clone, Debug)] -pub(crate) struct WeakMetadata { +pub(crate) struct WeakCounterMarker { weak_counter: Cell, } pub(crate) struct OverflowError; -impl WeakMetadata { +impl WeakCounterMarker { #[inline] #[must_use] - pub(crate) fn new(accessible: bool) -> WeakMetadata { - WeakMetadata { + pub(crate) fn new(accessible: bool) -> WeakCounterMarker { + WeakCounterMarker { weak_counter: Cell::new(if accessible { INITIAL_VALUE_ACCESSIBLE } else { diff --git a/tests/auto_collect.rs b/tests/auto_collect.rs index 2fb1964..76297d5 100644 --- a/tests/auto_collect.rs +++ b/tests/auto_collect.rs @@ -118,16 +118,16 @@ fn test_buffered_threshold_auto_collect() { _t: T, } - unsafe impl Trace for Cyclic { + unsafe impl Trace for Cyclic { fn trace(&self, ctx: &mut Context<'_>) { self.cyclic.trace(ctx); } } - impl Finalize for Cyclic { + impl Finalize for Cyclic { } - fn new() -> Cc> { + fn new() -> Cc> { let cc = Cc::new(Cyclic { cyclic: RefCell::new(None), _t: Default::default(), diff --git a/tests/weak_upgrade_tests.rs b/tests/weak_upgrade_tests.rs index e9806cd..cb2480c 100644 --- a/tests/weak_upgrade_tests.rs +++ b/tests/weak_upgrade_tests.rs @@ -1,5 +1,5 @@ #![cfg(not(miri))] // This test leaks memory, so it can't be run by Miri -#![cfg(all(feature = "weak-ptr", feature = "derive"))] +#![cfg(all(feature = "weak-ptrs", feature = "derive"))] //! This module tests that `Weak::upgrade` returns None when called in destructors while collecting. use std::cell::RefCell; @@ -14,7 +14,7 @@ struct Ignored { should_panic: bool, } -impl Drop for Ignored { +impl Drop for Ignored { fn drop(&mut self) { // This is safe to implement assert!(self.weak.upgrade().is_none()); @@ -57,7 +57,7 @@ fn cyclic_upgrade(should_panic: bool) { struct Allocated { #[rust_cc(ignore)] _ignored: Ignored, - cyclic: RefCell>>, + cyclic: RefCell>>, } let cc1 = Cc::new_cyclic(|weak| Allocated { @@ -101,6 +101,6 @@ fn cyclic_upgrade(should_panic: bool) { assert_eq!(should_panic, res.is_err()); assert!(weak1.upgrade().is_none()); - assert!(weak2.upgrade().is_none()); // This fails now |o| + assert!(weak2.upgrade().is_none()); assert!(weak3.upgrade().is_none()); }