Merge #153

153: x86_64: "practically" thread-safe  Pool r=korken89 a=japaric

### Summary / motivation

this PR makes `Pool` `Sync` on x86_64 with the main goal of being able to test `no_std` code that uses `Pool` (e.g. via the singleton `pool!` macro) on x86_64 (as it's not straight forward to run tests on embedded devices, and even less to CI those tests).

### Details

this PR reduces the chance of Pool running into the ABA problem (which corrupts the underlying lock-free stack) by using 32-bit version tags (search term: IBM ABA-prevention tags). Version tags do not 100% prevent the ABA problem but make it almost impossible to run into it in practice (the bigger the tag, in bits, the less the chance of running into ABA). See module level docs for details and limitations.

As this does not eliminate ABA with 100% certainty perhaps it should live behind a Cargo feature?

It seems to me that hazard pointers may be able to completely get rid of ABA but implementing those, AFAICT, require Thread Local Storage (`#[thread_local]`) which is not supported in `no_std` code.

r? @korken89 

Co-authored-by: Jorge Aparicio <jorge@japaric.io>

Author: bors[bot]

Cargo.toml

@@ -23,6 +23,8 @@ version = "0.5.4"
 default = ["cas"]
 cas = []
 ufmt-impl = ["ufmt-write"]
+# read the docs before enabling: makes `Pool` Sync on x86_64
+x86-sync-pool = []
 # only for tests
 __trybuild = []
 

ci/script.sh

@@ -5,8 +5,8 @@ main() {
     cargo check --target $TARGET --features 'serde'
 
     if [ $TARGET = x86_64-unknown-linux-gnu ]; then
-        cargo test --target $TARGET --features 'serde'
-        cargo test --target $TARGET --release --features 'serde'
+        cargo test --test cpass --target $TARGET --features 'serde'
+        cargo test --test cpass --target $TARGET --release --features 'serde'
 
         if [ $MSRV = 1 ]; then
             cd cfail
@@ -17,11 +17,10 @@ main() {
 
         if [ $TRAVIS_RUST_VERSION = nightly ]; then
             export RUSTFLAGS="-Z sanitizer=thread"
-            export RUST_TEST_THREADS=1
             export TSAN_OPTIONS="suppressions=$(pwd)/suppressions.txt"
 
-            cargo test --test tsan --target $TARGET
-            cargo test --test tsan --target $TARGET --release
+            cargo test --test tsan --features x86-sync-pool --target $TARGET
+            cargo test --test tsan --features x86-sync-pool --target $TARGET --release
         fi
     fi
 }

src/pool/cas.rs

@@ -0,0 +1,209 @@
+//! Stack based on CAS atomics
+//!
+//! To reduce the chance of hitting the ABA problem we use a 32-bit offset + a 32-bit version tag
+//! instead of a 64-bit pointer. The version tag will be bumped on each successful `pop` operation.
+
+use core::{
+    cell::UnsafeCell,
+    convert::TryFrom,
+    marker::PhantomData,
+    mem,
+    num::NonZeroUsize,
+    ptr::NonNull,
+    sync::atomic::{AtomicUsize, Ordering},
+};
+
+/// Unfortunate implementation detail required to use the
+/// [`Pool.grow_exact`](struct.Pool.html#method.grow_exact) method
+pub struct Node<T> {
+    next: Atomic<Node<T>>,
+    pub(crate) data: UnsafeCell<T>,
+}
+
+impl<T> Node<T> {
+    fn next(&self) -> &Atomic<Node<T>> {
+        &self.next
+    }
+}
+
+pub struct Stack<T> {
+    head: Atomic<Node<T>>,
+}
+
+impl<T> Stack<T> {
+    pub const fn new() -> Self {
+        Self {
+            head: Atomic::null(),
+        }
+    }
+
+    pub fn push(&self, new_head: Ptr<Node<T>>) {
+        let mut head = self.head.load(Ordering::Relaxed);
+
+        loop {
+            unsafe {
+                new_head
+                    .as_raw()
+                    .as_ref()
+                    .next()
+                    .store(head, Ordering::Relaxed);
+            }
+
+            if let Err(p) = self.head.compare_and_exchange_weak(
+                head,
+                Some(new_head),
+                Ordering::Release,
+                Ordering::Relaxed,
+            ) {
+                head = p;
+            } else {
+                return;
+            }
+        }
+    }
+
+    pub fn try_pop(&self) -> Option<Ptr<Node<T>>> {
+        loop {
+            if let Some(mut head) = self.head.load(Ordering::Acquire) {
+                let next = unsafe { head.as_raw().as_ref().next().load(Ordering::Relaxed) };
+
+                if self
+                    .head
+                    .compare_and_exchange_weak(
+                        Some(head),
+                        next,
+                        Ordering::Release,
+                        Ordering::Relaxed,
+                    )
+                    .is_ok()
+                {
+                    head.incr_tag();
+                    return Some(head);
+                }
+            } else {
+                // stack observed empty
+                return None;
+            }
+        }
+    }
+}
+
+fn anchor<T>() -> *mut T {
+    static mut ANCHOR: u8 = 0;
+    (unsafe { &mut ANCHOR } as *mut u8 as usize & !(mem::align_of::<T>() - 1)) as *mut T
+}
+
+/// Anchored pointer. This is a (signed) 32-bit offset from `anchor` plus a 32-bit tag
+pub struct Ptr<T> {
+    inner: NonZeroUsize,
+    _marker: PhantomData<*mut T>,
+}
+
+impl<T> Clone for Ptr<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T> Copy for Ptr<T> {}
+
+impl<T> Ptr<T> {
+    pub fn new(p: *mut T) -> Option<Self> {
+        i32::try_from((p as isize).wrapping_sub(anchor::<T>() as isize))
+            .ok()
+            .map(|offset| unsafe { Ptr::from_parts(0, offset) })
+    }
+
+    unsafe fn from_parts(tag: u32, offset: i32) -> Self {
+        Self {
+            inner: NonZeroUsize::new_unchecked((tag as usize) << 32 | (offset as u32 as usize)),
+            _marker: PhantomData,
+        }
+    }
+
+    fn from_usize(p: usize) -> Option<Self> {
+        NonZeroUsize::new(p).map(|inner| Self {
+            inner,
+            _marker: PhantomData,
+        })
+    }
+
+    fn into_usize(&self) -> usize {
+        self.inner.get()
+    }
+
+    fn tag(&self) -> u32 {
+        (self.inner.get() >> 32) as u32
+    }
+
+    fn incr_tag(&mut self) {
+        let tag = self.tag().wrapping_add(1);
+        let offset = self.offset();
+
+        *self = unsafe { Ptr::from_parts(tag, offset) };
+    }
+
+    fn offset(&self) -> i32 {
+        self.inner.get() as i32
+    }
+
+    fn as_raw(&self) -> NonNull<T> {
+        unsafe {
+            NonNull::new_unchecked(
+                (anchor::<T>() as *mut u8).offset(self.offset() as isize) as *mut T
+            )
+        }
+    }
+
+    pub fn dangling() -> Self {
+        unsafe { Self::from_parts(0, 1) }
+    }
+
+    pub unsafe fn as_ref(&self) -> &T {
+        &*self.as_raw().as_ptr()
+    }
+}
+
+struct Atomic<T> {
+    inner: AtomicUsize,
+    _marker: PhantomData<*mut T>,
+}
+
+impl<T> Atomic<T> {
+    const fn null() -> Self {
+        Self {
+            inner: AtomicUsize::new(0),
+            _marker: PhantomData,
+        }
+    }
+
+    fn compare_and_exchange_weak(
+        &self,
+        current: Option<Ptr<T>>,
+        new: Option<Ptr<T>>,
+        succ: Ordering,
+        fail: Ordering,
+    ) -> Result<(), Option<Ptr<T>>> {
+        self.inner
+            .compare_exchange_weak(
+                current.map(|p| p.into_usize()).unwrap_or(0),
+                new.map(|p| p.into_usize()).unwrap_or(0),
+                succ,
+                fail,
+            )
+            .map(drop)
+            .map_err(Ptr::from_usize)
+    }
+
+    fn load(&self, ord: Ordering) -> Option<Ptr<T>> {
+        NonZeroUsize::new(self.inner.load(ord)).map(|inner| Ptr {
+            inner,
+            _marker: PhantomData,
+        })
+    }
+
+    fn store(&self, val: Option<Ptr<T>>, ord: Ordering) {
+        self.inner
+            .store(val.map(|p| p.into_usize()).unwrap_or(0), ord)
+    }
+}

src/pool/llsc.rs

@@ -0,0 +1,78 @@
+//! Stack based on LL/SC atomics
+
+pub use core::ptr::NonNull as Ptr;
+use core::{
+    cell::UnsafeCell,
+    ptr,
+    sync::atomic::{AtomicPtr, Ordering},
+};
+
+/// Unfortunate implementation detail required to use the
+/// [`Pool.grow_exact`](struct.Pool.html#method.grow_exact) method
+pub struct Node<T> {
+    next: AtomicPtr<Node<T>>,
+    pub(crate) data: UnsafeCell<T>,
+}
+
+impl<T> Node<T> {
+    fn next(&self) -> &AtomicPtr<Node<T>> {
+        &self.next
+    }
+}
+
+pub struct Stack<T> {
+    head: AtomicPtr<Node<T>>,
+}
+
+impl<T> Stack<T> {
+    pub const fn new() -> Self {
+        Self {
+            head: AtomicPtr::new(ptr::null_mut()),
+        }
+    }
+
+    pub fn push(&self, new_head: Ptr<Node<T>>) {
+        // NOTE `Ordering`s come from crossbeam's (v0.6.0) `TreiberStack`
+
+        let mut head = self.head.load(Ordering::Relaxed);
+        loop {
+            unsafe { new_head.as_ref().next().store(head, Ordering::Relaxed) }
+
+            match self.head.compare_exchange_weak(
+                head,
+                new_head.as_ptr(),
+                Ordering::Release, // success
+                Ordering::Relaxed, // failure
+            ) {
+                Ok(_) => return,
+                // interrupt occurred or other core made a successful STREX op on the head
+                Err(p) => head = p,
+            }
+        }
+    }
+
+    pub fn try_pop(&self) -> Option<Ptr<Node<T>>> {
+        // NOTE `Ordering`s come from crossbeam's (v0.6.0) `TreiberStack`
+
+        loop {
+            let head = self.head.load(Ordering::Acquire);
+            if let Some(nn_head) = Ptr::new(head) {
+                let next = unsafe { nn_head.as_ref().next().load(Ordering::Relaxed) };
+
+                match self.head.compare_exchange_weak(
+                    head,
+                    next,
+                    Ordering::Release, // success
+                    Ordering::Relaxed, // failure
+                ) {
+                    Ok(_) => break Some(nn_head),
+                    // interrupt occurred or other core made a successful STREX op on the head
+                    Err(_) => continue,
+                }
+            } else {
+                // stack is observed as empty
+                break None;
+            }
+        }
+    }
+}