Skip to content

Commit

Permalink
add a phantom lifetime to ItemSliceSend
Browse files Browse the repository at this point in the history
This prevents mutating the items concurrently with the
`ItemSliceSend`. For example, before this patch, one could create two
`ItemSliceSend` from the same mutable slice.
  • Loading branch information
martinvonz committed Nov 16, 2023
1 parent 17a7c6a commit 6e5ef3b
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
6 changes: 3 additions & 3 deletions gix-pack/src/cache/delta/traverse/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ use crate::{
data::EntryRange,
};

pub(crate) struct State<F, MBFN, T: Send> {
pub(crate) struct State<'items, F, MBFN, T: Send> {
pub delta_bytes: Vec<u8>,
pub fully_resolved_delta_bytes: Vec<u8>,
pub progress: Box<dyn Progress>,
pub resolve: F,
pub modify_base: MBFN,
pub child_items: ItemSliceSend<Item<T>>,
pub child_items: ItemSliceSend<'items, Item<T>>,
}

#[allow(clippy::too_many_arguments)]
Expand All @@ -38,7 +38,7 @@ pub(crate) fn deltas<T, F, MBFN, E, R>(
resolve,
modify_base,
child_items,
}: &mut State<F, MBFN, T>,
}: &mut State<'_, F, MBFN, T>,
resolve_data: &R,
hash_len: usize,
threads_left: &AtomicIsize,
Expand Down
32 changes: 22 additions & 10 deletions gix-pack/src/cache/delta/traverse/util.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,49 @@
use std::marker::PhantomData;

use crate::cache::delta::Item;

pub struct ItemSliceSend<T>(*mut T)
pub struct ItemSliceSend<'a, T>
where
T: Send;
T: Send,
{
items: *mut T,
phantom: PhantomData<&'a T>,
}

impl<T> ItemSliceSend<T>
impl<'a, T> ItemSliceSend<'a, T>
where
T: Send,
{
pub fn new(items: &mut [T]) -> Self {
ItemSliceSend(items.as_mut_ptr())
pub fn new(items: &'a mut [T]) -> Self {
ItemSliceSend {
items: items.as_mut_ptr(),
phantom: PhantomData,
}
}
}

/// SAFETY: This would be unsafe if this would ever be abused, but it's used internally and only in a way that assure that the pointers
/// don't violate aliasing rules.
impl<T> Clone for ItemSliceSend<T>
impl<T> Clone for ItemSliceSend<'_, T>
where
T: Send,
{
fn clone(&self) -> Self {
ItemSliceSend(self.0)
ItemSliceSend {
items: self.items,
phantom: self.phantom,
}
}
}

// SAFETY: T is `Send`, and we only ever access one T at a time. And, ptrs need that assurance, I wonder if it's always right.
#[allow(unsafe_code)]
unsafe impl<T> Send for ItemSliceSend<T> where T: Send {}
unsafe impl<T> Send for ItemSliceSend<'_, T> where T: Send {}

/// An item returned by `iter_root_chunks`, allowing access to the `data` stored alongside nodes in a [`Tree`].
pub struct Node<'a, T: Send> {
pub item: &'a mut Item<T>,
pub child_items: ItemSliceSend<Item<T>>,
pub child_items: ItemSliceSend<'a, Item<T>>,
}

impl<'a, T: Send> Node<'a, T> {
Expand Down Expand Up @@ -66,7 +78,7 @@ impl<'a, T: Send> Node<'a, T> {
// SAFETY: The resulting mutable pointer cannot be yielded by any other node.
#[allow(unsafe_code)]
Node {
item: unsafe { &mut *children.0.add(index as usize) },
item: unsafe { &mut *children.items.add(index as usize) },
child_items: children.clone(),
}
})
Expand Down

0 comments on commit 6e5ef3b

Please sign in to comment.