Use Acq/Rel ordering everywhere

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "atom"
-version = "0.4.0"
+version = "0.5.0"
 authors = ["Colin Sherratt <[email protected]>"]
 license = "Apache-2.0"
 homepage = "https://github.com/slide-rs/atom"

examples/fifo.rs

@@ -16,7 +16,7 @@ extern crate atom;
 
 use atom::*;
 use std::mem;
-use std::sync::{atomic::Ordering, Arc, Barrier};
+use std::sync::{Arc, Barrier};
 use std::thread;
 
 const THREADS: usize = 100;
@@ -29,7 +29,7 @@ struct Link {
 impl Drop for Link {
     fn drop(&mut self) {
         // This is done to avoid a recusive drop of the List
-        while let Some(mut h) = self.next.atom().take(Ordering::Acquire) {
+        while let Some(mut h) = self.next.atom().take() {
             self.next = mem::replace(&mut h.next, AtomSetOnce::empty());
         }
     }
@@ -54,12 +54,12 @@ fn main() {
                 });
 
                 loop {
-                    while let Some(h) = hptr.next.get(Ordering::Acquire) {
+                    while let Some(h) = hptr.next.get() {
                         hptr = h;
                     }
 
                     my_awesome_node =
-                        match hptr.next.set_if_none(my_awesome_node, Ordering::Release) {
+                        match hptr.next.set_if_none(my_awesome_node) {
                             Some(v) => v,
                             None => break,
                         };
@@ -73,7 +73,7 @@ fn main() {
 
     let mut hptr = &*head;
     let mut count = 0;
-    while let Some(h) = hptr.next.get(Ordering::Acquire) {
+    while let Some(h) = hptr.next.get() {
         hptr = h;
         count += 1;
     }

examples/simple.rs

@@ -1,23 +1,23 @@
 extern crate atom;
 
 use atom::*;
-use std::sync::{atomic::Ordering, Arc};
+use std::sync::{Arc};
 use std::thread;
 
 fn main() {
     // Create an empty atom
     let shared_atom = Arc::new(Atom::empty());
 
     // set the value 75
-    shared_atom.swap(Box::new(75), Ordering::AcqRel);
+    shared_atom.swap(Box::new(75));
 
     // Spawn a bunch of thread that will try and take the value
     let threads: Vec<thread::JoinHandle<()>> = (0..8)
         .map(|_| {
             let shared_atom = shared_atom.clone();
             thread::spawn(move || {
                 // Take the contents of the atom, only one will win the race
-                if let Some(v) = shared_atom.take(Ordering::Acquire) {
+                if let Some(v) = shared_atom.take() {
                     println!("I got it: {:?} :D", v);
                 } else {
                     println!("I did not get it :(");

src/lib.rs

@@ -24,6 +24,24 @@ use std::sync::Arc;
 
 /// An Atom wraps an AtomicPtr, it allows for safe mutation of an atomic
 /// into common Rust Types.
+///
+/// All methods of this type perform [acquire-release synchronization][1] as
+/// necessary to ensure that the referenced object can be accessed safely.
+///
+/// [1]: https://en.cppreference.com/w/cpp/atomic/memory_order#Release-Acquire_ordering
+///
+/// ```
+/// // Create an empty atom
+/// use std::sync::Arc;
+/// use atom::Atom;
+///
+/// let shared_atom = Arc::new(Atom::empty());
+///
+/// shared_atom.set_if_none(Box::new(42));
+/// let old_value = shared_atom.swap(Box::new(75));
+///
+/// assert_eq!(old_value, Some(Box::new(42)));
+/// ```
 pub struct Atom<P>
 where
     P: IntoRawPtr + FromRawPtr,
@@ -63,28 +81,28 @@ where
 
     /// Swap a new value into the Atom, This will try multiple
     /// times until it succeeds. The old value will be returned.
-    pub fn swap(&self, v: P, order: Ordering) -> Option<P> {
+    pub fn swap(&self, v: P) -> Option<P> {
         let new = v.into_raw();
-        let old = self.inner.swap(new, order);
+        let old = self.inner.swap(new, Ordering::AcqRel);
         unsafe { Self::inner_from_raw(old) }
     }
 
     /// Take the value of the Atom replacing it with null pointer
     /// Returning the contents. If the contents was a `null` pointer the
     /// result will be `None`.
-    pub fn take(&self, order: Ordering) -> Option<P> {
-        let old = self.inner.swap(ptr::null_mut(), order);
+    pub fn take(&self) -> Option<P> {
+        let old = self.inner.swap(ptr::null_mut(), Ordering::Acquire);
         unsafe { Self::inner_from_raw(old) }
     }
 
     /// This will do a `CAS` setting the value only if it is NULL
     /// this will return `None` if the value was written,
     /// otherwise a `Some(v)` will be returned, where the value was
     /// the same value that you passed into this function
-    pub fn set_if_none(&self, v: P, order: Ordering) -> Option<P> {
+    pub fn set_if_none(&self, v: P) -> Option<P> {
         let new = v.into_raw();
-        let old = self.inner.compare_and_swap(ptr::null_mut(), new, order);
-        if !old.is_null() {
+        let result = self.inner.compare_exchange(ptr::null_mut(), new, Ordering::Release, Ordering::Relaxed);
+        if result.is_err() {
             Some(unsafe { FromRawPtr::from_raw(new) })
         } else {
             None
@@ -98,8 +116,6 @@ where
     pub fn replace_and_set_next(
         &self,
         mut value: P,
-        load_order: Ordering,
-        cas_order: Ordering,
     ) -> bool
     where
         P: GetNextMut<NextPtr = Option<P>>,
@@ -110,12 +126,13 @@ where
         // assert that it was droppeds
         unsafe { ptr::drop_in_place(next) };
         loop {
-            let pcurrent = self.inner.load(load_order);
+            let pcurrent = self.inner.load(Ordering::Acquire);
             let current = unsafe { Self::inner_from_raw(pcurrent) };
             unsafe { ptr::write(next, current) };
-            let last = self.inner.compare_and_swap(pcurrent, raw, cas_order);
-            if last == pcurrent {
-                return last.is_null();
+            let result = self.inner.compare_exchange(pcurrent, raw, Ordering::Release, Ordering::Relaxed);
+            match result {
+                Ok(replaced_ptr) => return replaced_ptr.is_null(),
+                _ => {}
             }
         }
     }
@@ -156,22 +173,17 @@ where
     /// returned together with a raw pointer to the Atom's current unchanged
     /// value, which is **not safe to dereference**, especially if the Atom is
     /// accessed from multiple threads.
-    ///
-    /// `compare_and_swap` also takes an `Ordering` argument which describes
-    /// the memory ordering of this operation.
     pub fn compare_and_swap(
         &self,
         current: Option<&P>,
         new: Option<P>,
-        order: Ordering,
     ) -> Result<Option<P>, (Option<P>, *mut P)> {
         let pcurrent = Self::inner_as_ptr(current);
         let pnew = Self::inner_into_raw(new);
-        let pprev = self.inner.compare_and_swap(pcurrent, pnew, order);
-        if pprev == pcurrent {
-            Ok(unsafe { Self::inner_from_raw(pprev) })
-        } else {
-            Err((unsafe { Self::inner_from_raw(pnew) }, pprev as *mut P))
+        let pprev = self.inner.compare_exchange(pcurrent, pnew, Ordering::AcqRel, Ordering::Acquire);
+        match pprev {
+            Ok(pprev) => Ok(unsafe { Self::inner_from_raw(pprev) }),
+            Err(pprev) => Err((unsafe { Self::inner_from_raw(pnew) }, pprev as *mut P))
         }
     }
 
@@ -181,23 +193,14 @@ where
     /// The return value is a result indicating whether the new value was
     /// written and containing the previous value. On success this value is
     /// guaranteed to be equal to `current`.
-    ///
-    /// `compare_exchange` takes two `Ordering` arguments to describe the
-    /// memory ordering of this operation. The first describes the required
-    /// ordering if the operation succeeds while the second describes the
-    /// required ordering when the operation fails. The failure ordering can't
-    /// be `Release` or `AcqRel` and must be equivalent or weaker than the
-    /// success ordering.
     pub fn compare_exchange(
         &self,
         current: Option<&P>,
         new: Option<P>,
-        success: Ordering,
-        failure: Ordering,
     ) -> Result<Option<P>, (Option<P>, *mut P)> {
         let pnew = Self::inner_into_raw(new);
         self.inner
-            .compare_exchange(Self::inner_as_ptr(current), pnew, success, failure)
+            .compare_exchange(Self::inner_as_ptr(current), pnew, Ordering::AcqRel, Ordering::Acquire)
             .map(|pprev| unsafe { Self::inner_from_raw(pprev) })
             .map_err(|pprev| (unsafe { Self::inner_from_raw(pnew) }, pprev as *mut P))
     }
@@ -209,23 +212,14 @@ where
     /// even when the comparison succeeds, which can result in more efficient
     /// code on some platforms. The return value is a result indicating whether
     /// the new value was written and containing the previous value.
-    ///
-    /// `compare_exchange_weak` takes two `Ordering` arguments to describe the
-    /// memory ordering of this operation. The first describes the required
-    /// ordering if the operation succeeds while the second describes the
-    /// required ordering when the operation fails. The failure ordering can't
-    /// be `Release` or `AcqRel` and must be equivalent or weaker than the
-    /// success ordering.
     pub fn compare_exchange_weak(
         &self,
         current: Option<&P>,
         new: Option<P>,
-        success: Ordering,
-        failure: Ordering,
     ) -> Result<Option<P>, (Option<P>, *mut P)> {
         let pnew = Self::inner_into_raw(new);
         self.inner
-            .compare_exchange_weak(Self::inner_as_ptr(current), pnew, success, failure)
+            .compare_exchange_weak(Self::inner_as_ptr(current), pnew, Ordering::AcqRel, Ordering::Acquire)
             .map(|pprev| unsafe { Self::inner_from_raw(pprev) })
             .map_err(|pprev| (unsafe { Self::inner_from_raw(pnew) }, pprev as *mut P))
     }
@@ -244,7 +238,7 @@ where
     P: IntoRawPtr + FromRawPtr,
 {
     fn drop(&mut self) {
-        self.take(Ordering::Relaxed);
+        self.take();
     }
 }
 
@@ -360,8 +354,8 @@ where
     /// this will return `OK(())` if the value was written,
     /// otherwise a `Err(P)` will be returned, where the value was
     /// the same value that you passed into this function
-    pub fn set_if_none(&self, v: P, order: Ordering) -> Option<P> {
-        self.inner.set_if_none(v, order)
+    pub fn set_if_none(&self, v: P) -> Option<P> {
+        self.inner.set_if_none(v)
     }
 
     /// Convert an `AtomSetOnce` into an `Atom`
@@ -387,8 +381,8 @@ where
     P: IntoRawPtr + FromRawPtr + Deref<Target = T>,
 {
     /// If the Atom is set, get the value
-    pub fn get(&self, order: Ordering) -> Option<&T> {
-        let ptr = self.inner.inner.load(order);
+    pub fn get(&self) -> Option<&T> {
+        let ptr = self.inner.inner.load(Ordering::Acquire);
         let val = unsafe { Atom::inner_from_raw(ptr) };
         val.map(|v: P| {
             // This is safe since ptr cannot be changed once it is set
@@ -402,8 +396,8 @@ where
 
 impl<T> AtomSetOnce<Box<T>> {
     /// If the Atom is set, get the value
-    pub fn get_mut(&mut self, order: Ordering) -> Option<&mut T> {
-        let ptr = self.inner.inner.load(order);
+    pub fn get_mut(&mut self) -> Option<&mut T> {
+        let ptr = *self.inner.inner.get_mut();
         let val = unsafe { Atom::inner_from_raw(ptr) };
         val.map(move |mut v: Box<T>| {
             // This is safe since ptr cannot be changed once it is set
@@ -420,8 +414,8 @@ where
     T: Clone + IntoRawPtr + FromRawPtr,
 {
     /// Duplicate the inner pointer if it is set
-    pub fn dup(&self, order: Ordering) -> Option<T> {
-        let ptr = self.inner.inner.load(order);
+    pub fn dup(&self) -> Option<T> {
+        let ptr = self.inner.inner.load(Ordering::Acquire);
         let val = unsafe { Atom::inner_from_raw(ptr) };
         val.map(|v: T| {
             let out = v.clone();