Skip to content

Commit

Permalink
Was missing ?Sized-bound on Sync-impl (#7)
Browse files Browse the repository at this point in the history 
* Was missing ?Sized-bound on Sync-impl

* Add ?Sized-bounds and make ArcShift actually work for unsized payloads
  • Loading branch information
@avl
avl authored Oct 2, 2024
1 parent 67ba3bd commit 9c63bac
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 103 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ this line to the cargo mutants-config.

Another challenge is how to handle overflow of ArcShift-instances. The problem is that cargo
mutants identifies that removing the code for handling ArcShift instance count overflow doesn't
fail the test suite. However, triggering such an overflow would require creating 2*45 instances
fail the test suite. However, triggering such an overflow would require creating 2^45 instances
of ArcShift. Unfortunately, this requires at least 280 TB of RAM.

In the end, I just added an exception for this logic. Sanity checks which are not expected
Expand Down
2 changes: 1 addition & 1 deletion arcshift/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "arcshift"
version = "0.1.9"
version = "0.1.10"
documentation = "https://docs.rs/arcshift/"
homepage = "https://github.com/avl/arcshift/"
repository = "https://github.com/avl/arcshift/"
Expand Down
230 changes: 129 additions & 101 deletions arcshift/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,31 @@ pub struct ArcShift<T: 'static + ?Sized> {
item: NonNull<ItemHolderDummy<T>>,
}

impl<T: ?Sized + 'static> Debug for ArcShift<T>
where
T: Debug,
{
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "ArcShift({:?})", self.shared_get())
}
}
impl<T: ?Sized + 'static> Debug for ArcShiftLight<T>
where
T: Debug,
{
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "ArcShiftLight(..)")
}
}
impl<T: ?Sized + 'static> Debug for ArcShiftCell<T>
where
T: Debug,
{
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "ArcShiftCell({:?})", &*self.borrow())
}
}

impl<T> UnwindSafe for ArcShift<T> {}

/// ArcShiftLight is like ArcShift, except it does not provide overhead-free access.
Expand Down Expand Up @@ -344,12 +369,12 @@ pub struct ArcShiftLight<T: 'static + ?Sized> {
/// SAFETY:
/// ArcShiftLight can be Send as long as T is Send.
/// ArcShiftLight's mechanisms are compatible with both Send and Sync
unsafe impl<T: 'static> Send for ArcShiftLight<T> where T: Send {}
unsafe impl<T: 'static + ?Sized> Send for ArcShiftLight<T> where T: Send {}

/// SAFETY:
/// ArcShiftLight can be Sync as long as T is Sync.
/// ArcShiftLight's mechanisms are compatible with both Send and Sync
unsafe impl<T: 'static> Sync for ArcShiftLight<T> where T: Sync {}
unsafe impl<T: 'static + ?Sized> Sync for ArcShiftLight<T> where T: Sync {}

/// ArcShiftCell is like an ArcShift, except that it can be reloaded
/// without requiring 'mut'-access.
Expand All @@ -361,7 +386,7 @@ unsafe impl<T: 'static> Sync for ArcShiftLight<T> where T: Sync {}
/// but if it is leaked, the effect is that whatever value the ArcShiftCell-instance
/// pointed to at that time, will forever leak also. All the linked-list nodes from
/// that entry and onward will also leak. So make sure to not leak the handle!
pub struct ArcShiftCell<T: 'static> {
pub struct ArcShiftCell<T: 'static + ?Sized> {
inner: UnsafeCell<ArcShift<T>>,
recursion: Cell<usize>,
}
Expand All @@ -372,11 +397,11 @@ pub struct ArcShiftCell<T: 'static> {
/// that entry and onward will also leak. So make sure to not leak the handle!
///
/// Leaking the handle does not cause unsoundness and is not UB.
pub struct ArcShiftCellHandle<'a, T: 'static> {
pub struct ArcShiftCellHandle<'a, T: 'static + ?Sized> {
cell: &'a ArcShiftCell<T>,
}

impl<'a, T: 'static> Drop for ArcShiftCellHandle<'a, T> {
impl<'a, T: 'static + ?Sized> Drop for ArcShiftCellHandle<'a, T> {
fn drop(&mut self) {
let mut rec = self.cell.recursion.get();
rec -= 1;
Expand All @@ -390,7 +415,7 @@ impl<'a, T: 'static> Drop for ArcShiftCellHandle<'a, T> {
}
}

impl<'a, T: 'static> Deref for ArcShiftCellHandle<'a, T> {
impl<'a, T: 'static + ?Sized> Deref for ArcShiftCellHandle<'a, T> {
type Target = T;

fn deref(&self) -> &Self::Target {
Expand Down Expand Up @@ -422,7 +447,7 @@ impl<'a, T: 'static> Deref for ArcShiftCellHandle<'a, T> {
/// could call 'get' simultaneously, corrupting the (non-atomic) refcount.
unsafe impl<T: 'static> Send for ArcShiftCell<T> where T: Send {}

impl<T: 'static> Clone for ArcShiftCell<T> {
impl<T: 'static + ?Sized> Clone for ArcShiftCell<T> {
fn clone(&self) -> Self {
// SAFETY:
// Accessing the inner value is safe, since we know no other thread
Expand Down Expand Up @@ -459,7 +484,8 @@ impl<T: 'static> ArcShiftCell<T> {
pub fn new(value: T) -> ArcShiftCell<T> {
ArcShiftCell::from_arcshift(ArcShift::new(value))
}

}
impl<T: 'static + ?Sized> ArcShiftCell<T> {
/// Creates an ArcShiftCell from an ArcShift-instance.
/// The payload is not cloned, the two pointers keep pointing to the same object.
pub fn from_arcshift(input: ArcShift<T>) -> ArcShiftCell<T> {
Expand Down Expand Up @@ -550,7 +576,7 @@ const fn is_sized<T: ?Sized>() -> bool {
size_of::<&T>() == size_of::<&()>()
}

impl<T: 'static> Clone for ArcShiftLight<T> {
impl<T: 'static + ?Sized> Clone for ArcShiftLight<T> {
fn clone(&self) -> Self {
let mut curitem = self.item.as_ptr() as *const _;
loop {
Expand Down Expand Up @@ -956,6 +982,91 @@ impl<T: 'static + ?Sized> ArcShiftLight<T> {
Some(next)
}
}

/// Create an ArcShift instance from this ArcShiftLight.
pub fn upgrade(&self) -> ArcShift<T> {
debug_println!("ArcShiftLight Promoting to ArcShift {:?}", self.item);
let mut curitem = self.item.as_ptr();
loop {
let Some(next) = Self::load_nontentative_next(curitem) else {
atomic::spin_loop();
continue;
};

debug_println!(
"ArcShiftLight upgrade {:?}, next: {:?} = {:?}",
curitem,
next,
get_state(next)
);

if !next.is_null() {
debug_println!(
"ArcShiftLight traversing chain to {:?} -> {:?}",
curitem,
next
);
curitem = undecorate(next) as *mut _;
atomic::spin_loop();
continue;
}

let precount = get_refcount(curitem).fetch_add(MAX_ROOTS, Ordering::SeqCst);
if precount >= MAX_ARCSHIFT {
let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
panic!(
"Maximum supported ArcShift instance count reached: {}",
MAX_ARCSHIFT
);
}
atomic::fence(Ordering::SeqCst); //Just to make loom work
debug_println!(
"Promote {:?}, prev count: {}, new count {}",
curitem,
precount,
precount + MAX_ROOTS
);
assert!(precount >= 1);
let Some(next) = Self::load_nontentative_next(curitem) else {
let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
assert!(_precount > MAX_ROOTS && _precount < 1_000_000_000_000);
atomic::spin_loop();
continue;
};
if !undecorate(next).is_null() {
debug_println!(
"ArcShiftLight About to reduce count {:?} by {}, and traversing to {:?}",
curitem,
MAX_ROOTS,
next
);

let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
assert!(_precount > MAX_ROOTS && _precount < 1_000_000_000_000);
curitem = undecorate(next) as *mut _;
atomic::spin_loop();
continue;
}

let mut temp = ArcShift { item:
// SAFETY:
// curitem cannot be null, since it is either the value of 'self.item', which
// cannot be null, or assigned by code above that first checks for null
unsafe { NonNull::new_unchecked(curitem) }
};
temp.reload();
return temp;
}
}
#[cfg_attr(test, mutants::skip)]
fn verify_count(count: usize) {
if (count & (MAX_ROOTS - 1)) >= MAX_ROOTS - 1 {
panic!(
"Max limit of ArcShiftLight clones ({}) was reached",
MAX_ROOTS
);
}
}
}

impl<T: 'static> ArcShiftLight<T> {
Expand Down Expand Up @@ -1048,16 +1159,6 @@ impl<T: 'static> ArcShiftLight<T> {
}
}

#[cfg_attr(test, mutants::skip)]
fn verify_count(count: usize) {
if (count & (MAX_ROOTS - 1)) >= MAX_ROOTS - 1 {
panic!(
"Max limit of ArcShiftLight clones ({}) was reached",
MAX_ROOTS
);
}
}

/// Update the contents of this ArcShift, and all other instances cloned from this
/// instance. The next time such an instance of ArcShift is dereferenced, this
/// new value will be returned.
Expand Down Expand Up @@ -1186,82 +1287,6 @@ impl<T: 'static> ArcShiftLight<T> {
let new_ptr = ArcShift::from_box_impl(new_payload);
ArcShift::update_shared_impl(self.item.as_ptr(), new_ptr as *mut _, true, null());
}

/// Create an ArcShift instance from this ArcShiftLight.
pub fn upgrade(&self) -> ArcShift<T> {
debug_println!("ArcShiftLight Promoting to ArcShift {:?}", self.item);
let mut curitem = self.item.as_ptr();
loop {
let Some(next) = Self::load_nontentative_next(curitem) else {
atomic::spin_loop();
continue;
};

debug_println!(
"ArcShiftLight upgrade {:?}, next: {:?} = {:?}",
curitem,
next,
get_state(next)
);

if !next.is_null() {
debug_println!(
"ArcShiftLight traversing chain to {:?} -> {:?}",
curitem,
next
);
curitem = undecorate(next) as *mut _;
atomic::spin_loop();
continue;
}

let precount = get_refcount(curitem).fetch_add(MAX_ROOTS, Ordering::SeqCst);
if precount >= MAX_ARCSHIFT {
let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
panic!(
"Maximum supported ArcShift instance count reached: {}",
MAX_ARCSHIFT
);
}
atomic::fence(Ordering::SeqCst); //Just to make loom work
debug_println!(
"Promote {:?}, prev count: {}, new count {}",
curitem,
precount,
precount + MAX_ROOTS
);
assert!(precount >= 1);
let Some(next) = Self::load_nontentative_next(curitem) else {
let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
assert!(_precount > MAX_ROOTS && _precount < 1_000_000_000_000);
atomic::spin_loop();
continue;
};
if !undecorate(next).is_null() {
debug_println!(
"ArcShiftLight About to reduce count {:?} by {}, and traversing to {:?}",
curitem,
MAX_ROOTS,
next
);

let _precount = get_refcount(curitem).fetch_sub(MAX_ROOTS, Ordering::SeqCst);
assert!(_precount > MAX_ROOTS && _precount < 1_000_000_000_000);
curitem = undecorate(next) as *mut _;
atomic::spin_loop();
continue;
}

let mut temp = ArcShift { item:
// SAFETY:
// curitem cannot be null, since it is either the value of 'self.item', which
// cannot be null, or assigned by code above that first checks for null
unsafe { NonNull::new_unchecked(curitem) }
};
temp.reload();
return temp;
}
}
}

impl<T: 'static + ?Sized> Drop for ArcShiftLight<T> {
Expand All @@ -1273,17 +1298,20 @@ impl<T: 'static + ?Sized> Drop for ArcShiftLight<T> {

/// SAFETY:
/// If `T` is `Sync`, `ArcShift<T>` can also be `Sync`
unsafe impl<T: 'static + Sync> Sync for ArcShift<T> {}
unsafe impl<T: 'static + Sync + ?Sized> Sync for ArcShift<T> {}

/// SAFETY:
/// If `T` is `Send`, `ArcShift<T>` can also be `Send`
unsafe impl<T: 'static + Send> Send for ArcShift<T> {}
unsafe impl<T: 'static + Send + ?Sized> Send for ArcShift<T> {}

impl<T: ?Sized> Drop for ArcShift<T> {
fn drop(&mut self) {
verify_item(get_ptr(self.item));

self.reload();
if !undecorate(get_next_and_state(self.item.as_ptr()).load(Ordering::Relaxed)).is_null() {
//Fast-path, when 'next' is null, don't do the expensive and complicated 'reload'
self.reload();
}
debug_println!("ArcShift::drop({:?}) - reloaded", self.item);
drop_item(get_ptr(self.item));
debug_println!("ArcShift::drop({:?}) DONE", self.item);
Expand Down Expand Up @@ -1539,7 +1567,7 @@ fn is_superseded_by_tentative(state: Option<ItemStateEnum>) -> bool {
matches!(state, Some(ItemStateEnum::SupersededByTentative))
}

impl<T: 'static> Clone for ArcShift<T> {
impl<T: 'static + ?Sized> Clone for ArcShift<T> {
fn clone(&self) -> Self {
debug_println!("ArcShift::clone({:?})", self.item);
let rescount =
Expand All @@ -1557,7 +1585,7 @@ impl<T: 'static> Clone for ArcShift<T> {
ArcShift { item: self.item }
}
}
impl<T> Deref for ArcShift<T> {
impl<T: ?Sized> Deref for ArcShift<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
Expand Down Expand Up @@ -1874,7 +1902,7 @@ impl<T: 'static + ?Sized> ArcShift<T> {
}

debug_println!("drop_impl to reduce count {:?} by {}", self_item, strength);
let count = get_refcount(self_item).fetch_sub(strength, atomic::Ordering::SeqCst);
let count = get_refcount(self_item).fetch_sub(strength, atomic::Ordering::AcqRel);
debug_println!(
"drop_impl on {:?} - count: {} -> {}",
self_item,
Expand Down

0 comments on commit 9c63bac

Please sign in to comment.