Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose asymmetric_light_barrier and add a document #56

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Expose asymmetric_light_barrier and add a document #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bcd84ec
Format codes and remove warnings on unused vars
powergee Nov 22, 2023
a69bc7f
Expose `asymmetric_light_barrier` and write a doc
powergee Nov 22, 2023
7e85d86
Fix typo and format
powergee Nov 22, 2023
5ba2f17
Correct some typos
powergee Dec 10, 2023
230d1fe
Correct minor typo: `which` to `that`
powergee Dec 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/domain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,8 +1041,8 @@ mod tests {
#[test]
fn create_multiple_unique_domains() {
use crate::Singleton;
let domain_1 = unique_domain!();
let domain_2 = unique_domain!();
let _domain_1 = unique_domain!();
let _domain_2 = unique_domain!();
}

#[test]
Expand Down
88 changes: 88 additions & 0 deletions src/hazard.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,94 @@ impl<'domain, F> HazardPointer<'domain, F> {
///
/// Note that protecting a given pointer only has an effect if any thread that may drop the
/// pointer does so through the same [`Domain`] as this hazard pointer is associated with.
///
/// It's important to note that this function solely writes the pointer value to the hazard
/// pointer slot. However, this protection alone does not guarantee safety during dereferencing
/// due to two key reasons:
///
/// 1. The announcement made by the hazard pointer might not be immediately visible to
/// reclaiming threads, especially in a weak memory model.
/// 2. Concurrent threads could already have retired the pointer before the protection.
///
/// To ensure safety, users need to appropriately synchronize the write operation on a hazard
/// slot and validate that the pointer hasn't already been retired. For synchronization, the
/// library offers an [`asymmetric_light_barrier`] function. It enables reclaiming threads
/// to acknowledge the preceding protection.
Comment on lines +251 to +254
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wording here is a little imprecise for my taste. The current phrasing makes it seem like an acknowledgement is somehow "sent" via a call to asymmetric_light_barrier, but that's not really what happens. My understanding is that the barrier specifically has two side-effects:

  1. That the load_ptr on head happens strictly after the call to protect_raw
  2. That if another thread exchanges head, and that exchange is not observed by this thread, then the protect_raw must be visible to that other thread (since the exchange must have happened-after our load_ptr, which happened after our protect_raw).

I think it would be good to capture some of that nuance here and/or in the docs for asymmetric_light_barrier.

What do you think?

Copy link
Author

@powergee powergee Dec 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing, and I am sorry for the late reply. 😅

I agree with your point. The current explanation might mislead readers that asymmetric_light_barrier alone somehow prevents reclaimers from freeing the object, which is not true. I think that I have omitted too many details.
The relationship between barriers and validation should be described more clearly.

The main idea that must be expressed would be that asymmetric_light_barrier makes essential happens-before relations to hold. For example, if I model the entire system of Hazard pointers like the following...

procedure Protect(p):
  P1. Announce protection of p.
  P2. Issue a **light** barrier.
  P3. Check if p is not retired. If retired, must retry after reloading the pointer.

procedure Reclaim(p):
  R1. Announce retirement of p.
  R2. Issue a **heavy** barrier.
  R3. Check if p is protected. If protected, retry later.

The semantics of the light and heavy barrier are (-> is happens-before relation):

  1. If P2 -> R2, P1 -> R3 (the protection of P1 is visible to R3).
  2. If R2 -> P2, R1 -> P3 (the retirement of R1 is visible to P3).
  3. P2 ensures P1 -> P2 -> P3 and R2 ensures R1 -> R2 -> R3.

These three lemmas cover the two points you mentioned. With these lemmas, we can show that:

  1. If P2 happens before R2, the reclaimer will not free p.
  2. If R2 happens before P2, the accessor will not access p.

In either case, use-after-free errors do not occur.

So far, this is what I'm going to describe in a revised version. Please feel free to give comments if you want.

I think I'd be able to push a revised version this week. After the work, I would be happy if you review my PR again!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that looks really good, thanks for diving so deep into it!

///
/// Manual pointer protection and validation involve the following steps:
///
/// 1. Acquire a pointer `p`, and manually protect it with a [`HazardPointer`] by calling
/// [`HazardPointer::protect_raw`].
/// 2. Issue a memory barrier with [`asymmetric_light_barrier`] to enable reclaiming threads
/// to recognize the preceding protection.
/// 3. Validate whether `p` is retired. If `p` remains guaranteed as not retired, it is safe
/// for dereferencing. Otherwise, revisit step 1 and retry.
///
/// The strategy to validate whether `p` is retired would depend on the semantics of
/// the data structures or algorithms. For example, in Harris-Michael linked lists, validation
/// can be done by reloading the pointer from the [`AtomicPtr`] and ensuring that its
/// value has not changed. This strategy works because unlinking the node from its predecessor
/// strictly *happens before* the retirement of that node under the data structure's semantics.
///
/// # Example
///
/// ```
/// use haphazard::{AtomicPtr, HazardPointer, asymmetric_light_barrier};
///
/// struct Node {
/// value: usize,
/// next: AtomicPtr<Self>,
/// }
///
/// // Let's imagine a data structure that has the following properties.
/// //
/// // 1. It always has exactly two nodes.
/// // 2. A thread may change its contents by exchanging the `head` pointer with another chain
/// // consisting of two nodes.
/// // 3. After a successful `compare_exchange`, the thread retires popped nodes without
/// // unlinking the first and the second node.
/// //
/// // Note that the link between the first and the second node won't be changed
/// // before the retirement! For this reason, to validate the protection of the second node,
/// // one must reload the head pointer and confirm that it has not changed.
/// let head =
/// AtomicPtr::from(Box::new(Node {
/// value: 0,
/// next: AtomicPtr::from(Box::new(Node {
/// value: 1,
/// next: unsafe { AtomicPtr::new(std::ptr::null_mut()) },
/// })),
/// }));
///
/// let mut hp1 = HazardPointer::default();
/// let mut hp2 = HazardPointer::default();
///
/// let (n1, n2) = loop {
/// // The first node can be loaded in a conventional way.
/// let n1 = head.safe_load(&mut hp1).expect("The first node must exist");
///
/// // However, the second one cannot, because of the aforementioned reasons.
/// let ptr = n1.next.load_ptr();
/// // 1. Announce a hazard pointer manually.
/// hp2.protect_raw(ptr);
///
/// // 2. Synchronize with reclaimers.
/// asymmetric_light_barrier();
///
/// // 3. Validate the second protection by reloading the head pointer.
/// if n1 as *const _ == head.load_ptr().cast_const() {
/// // If the link to the head node has not changed,
/// // it is guaranteed that the second node is not retired yet.
///
/// // Safety: `ptr` is properly protected by `hp2`.
/// let n2 = unsafe { &*ptr };
/// break (n1, n2);
/// }
///
/// };
///
/// // Here, `n1` and `n2` is safe for dereferencing.
/// ```
pub fn protect_raw<T>(&mut self, ptr: *mut T)
where
F: 'static,
Expand Down
13 changes: 12 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,18 @@ mod pointer;
mod record;
mod sync;

fn asymmetric_light_barrier() {
/// Issue a memory barrier to announce a protection to reclaiming threads.
///
/// In most cases, you do not need to use this function, because [`AtomicPtr::safe_load`] and all
/// protection methods of [`HazardPointer`] except [`HazardPointer::protect_raw`] properly protect
/// a pointer and internally call this function to validate protection.
///
/// However, in specific data structures or algorithms requiring manual pointer protection using
/// [`HazardPointer::protect_raw`], this function can be used to manually synchronize the memory
/// writes with reclaiming threads.
///
/// See also [`HazardPointer::protect_raw`].
pub fn asymmetric_light_barrier() {
// TODO: if cfg!(linux) {
// https://github.com/facebook/folly/blob/bd600cd4e88f664f285489c76b6ad835d8367cd2/folly/portability/Asm.h#L28
crate::sync::atomic::fence(core::sync::atomic::Ordering::SeqCst);
Expand Down
110 changes: 94 additions & 16 deletions tests/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,8 @@ fn acquires_multiple() {

let domain = Domain::new(&());

let x = AtomicPtr::new(Box::into_raw(Box::new((
42,
CountDrops(Arc::clone(&drops_42)),
))));
let y = AtomicPtr::new(Box::into_raw(Box::new((
42,
CountDrops(Arc::clone(&drops_42)),
))));
let x = AtomicPtr::new(Box::into_raw(Box::new((42, CountDrops(Arc::clone(&drops_42))))));
let y = AtomicPtr::new(Box::into_raw(Box::new((42, CountDrops(Arc::clone(&drops_42))))));

// As a reader:
let mut hazptr_array = HazardPointer::many_in_domain(&domain);
Expand Down Expand Up @@ -77,10 +71,7 @@ fn acquires_multiple() {
fn feels_good() {
let drops_42 = Arc::new(AtomicUsize::new(0));

let x = AtomicPtr::new(Box::into_raw(Box::new((
42,
CountDrops(Arc::clone(&drops_42)),
))));
let x = AtomicPtr::new(Box::into_raw(Box::new((42, CountDrops(Arc::clone(&drops_42))))));

// As a reader:
let mut h = HazardPointer::new();
Expand Down Expand Up @@ -166,10 +157,7 @@ fn drop_domain() {

let drops_42 = Arc::new(AtomicUsize::new(0));

let x = AtomicPtr::new(Box::into_raw(Box::new((
42,
CountDrops(Arc::clone(&drops_42)),
))));
let x = AtomicPtr::new(Box::into_raw(Box::new((42, CountDrops(Arc::clone(&drops_42))))));

// As a reader:
let mut h = HazardPointer::new_in_domain(&domain);
Expand Down Expand Up @@ -233,3 +221,93 @@ fn hazardptr_compare_exchange_fail() {

let _ = unsafe { Box::from_raw(not_current) };
}

#[test]
fn manual_validation() {
struct Node {
value: usize,
next: haphazard::AtomicPtr<Self>,
}

// Let's imagine a data structure which has the following properties.
//
// 1. It always has exactly two nodes.
// 2. A thread may change its contents by exchanging the `head` pointer with an another
// chain consisted of two nodes.
// 3. After a successful `compare_exchange`, the thread retires popped nodes without unlinking
// the first and the second node.
//
// Note that the link between the first and the second node won't be changed
// before the retirement! For this reason, to validate the protection for the second node,
// one must reload the head pointer and confirm that it has not changed.
let head =
haphazard::AtomicPtr::from(Box::new(Node {
value: 0,
next: haphazard::AtomicPtr::from(Box::new(Node {
value: 1,
next: unsafe { haphazard::AtomicPtr::new(std::ptr::null_mut()) },
})),
}));

const THREADS: usize = 8;
const ITERS: usize = 512;

std::thread::scope(|s| {
for _ in 0..THREADS {
s.spawn(|| {
let mut hp1 = HazardPointer::default();
let mut hp2 = HazardPointer::default();
for _ in 0..ITERS {
loop {
let (n1, n2) = loop {
// The first node can be loaded in a conventional way.
let n1 = head.safe_load(&mut hp1).expect("The first node must exist");

let ptr = n1.next.load_ptr();
hp2.protect_raw(ptr);

// Synchronize with reclaimers.
asymmetric_light_barrier();

// Validate the second protection by reloading the head pointer.
if n1 as *const _ == head.load_ptr().cast_const() {
// Safety: Because the head pointer did not change,
// the two nodes are not retired, and the previous
// protection is valid!
let n2 = unsafe { &*ptr };
break (n1, n2);
}
};

let next = Box::new(Node {
value: n1.value + 1,
next: haphazard::AtomicPtr::from(Box::new(Node {
value: n2.value + 1,
next: unsafe { haphazard::AtomicPtr::new(std::ptr::null_mut()) },
})),
});
if head
.compare_exchange(n1 as *const _ as *mut Node, next)
.is_ok()
{
// Safety: As we won the race of exchanging the head node,
// they have not already been retired.
unsafe {
Domain::global()
.retire_ptr::<_, Box<_>>(n1 as *const _ as *mut Node);
Domain::global()
.retire_ptr::<_, Box<_>>(n2 as *const _ as *mut Node);
}
break;
}
}
}
});
}
});

let n1 = unsafe { Box::from_raw(head.into_inner()) };
let n2 = unsafe { Box::from_raw(n1.next.into_inner()) };
assert_eq!(n1.value, THREADS * ITERS);
assert_eq!(n2.value, THREADS * ITERS + 1);
}