Skip to content

Commit

Permalink
tests: Re-add loom tests
Browse files Browse the repository at this point in the history
Mirrors #126

Signed-off-by: John Nunley <dev@notgull.net>
  • Loading branch information
notgull committed Apr 14, 2024
1 parent eba1ae5 commit bae7aef
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 47 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,14 @@ jobs:
- uses: rustsec/audit-check@master
with:
token: ${{ secrets.GITHUB_TOKEN }}

loom:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install Rust
run: rustup update stable
- name: Loom tests
run: RUSTFLAGS="--cfg=loom" cargo test --release --test loom --features loom
- name: Loom tests for lock-free
run: RUSTFLAGS="--cfg=loom" cargo test --no-default-features --release --test loom --features loom
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ futures-lite = "2.3.0"
criterion = { version = "0.3.4", default-features = false, features = ["cargo_bench_support"] }
waker-fn = "1"

[target.'cfg(loom)'.dependencies]
loom = { version = "0.7", optional = true }

[[bench]]
name = "bench"
harness = false
Expand Down
121 changes: 88 additions & 33 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@
#![no_std]
#![warn(missing_docs, missing_debug_implementations, rust_2018_idioms)]

#[cfg(any(not(feature = "std"), not(feature = "portable-atomic")))]
#[cfg(all(
not(all(feature = "std", loom)),
any(not(feature = "std"), not(feature = "portable-atomic"))
))]
extern crate alloc;

#[cfg(feature = "std")]
extern crate std;

use loom::atomic::{self, AtomicPtr, AtomicUsize, Ordering};
Expand All @@ -105,6 +107,22 @@ mod notify;
pub(crate) use linked_list::Inner;
pub use notify::{IntoNotification, Notification};

/// Make the given function const if the given condition is true.
macro_rules! const_fn {
(
const_if: #[cfg($($cfg:tt)+)];
$(#[$($attr:tt)*])*
$vis:vis const fn $($rest:tt)*
) => {
#[cfg($($cfg)+)]
$(#[$($attr)*])*
$vis const fn $($rest)*
#[cfg(not($($cfg)+))]
$(#[$($attr)*])*
$vis fn $($rest)*
};
}

/// Create a stack-based event listener for an [`Event`].
///
/// [`EventListener`] allocates the listener on the heap. While this works for most use cases, in
Expand Down Expand Up @@ -239,18 +257,21 @@ impl<T> UnwindSafe for Event<T> {}
impl<T> RefUnwindSafe for Event<T> {}

impl Event {
/// Creates a new [`Event`].
///
/// # Examples
///
/// ```
/// use event_listener::Event;
///
/// let event = Event::new();
/// ```
#[inline]
pub const fn new() -> Event {
Self::with_tag()
const_fn! {
const_if: #[cfg(not(loom))];
/// Creates a new [`Event`].
///
/// # Examples
///
/// ```
/// use event_listener::Event;
///
/// let event = Event::new();
/// ```
#[inline]
pub const fn new() -> Event {
Self::with_tag()
}
}

/// Notifies a number of active listeners without emitting a `SeqCst` fence.
Expand Down Expand Up @@ -368,19 +389,22 @@ impl Event {
}

impl<T> Event<T> {
/// Creates a new [`Event`] with a tag.
///
/// # Examples
///
/// ```
/// use event_listener::Event;
///
/// let event = Event::<usize>::with_tag();
/// ```
#[inline]
pub const fn with_tag() -> Event<T> {
Event {
inner: AtomicPtr::new(ptr::null_mut()),
const_fn! {
const_if: #[cfg(not(loom))];
/// Creates a new [`Event`] with a tag.
///
/// # Examples
///
/// ```
/// use event_listener::Event;
///
/// let event = Event::<usize>::with_tag();
/// ```
#[inline]
pub const fn with_tag() -> Event<T> {
Event {
inner: AtomicPtr::new(ptr::null_mut()),
}
}
}

Expand Down Expand Up @@ -565,14 +589,16 @@ impl<T> Event<T> {
impl<T> Drop for Event<T> {
#[inline]
fn drop(&mut self) {
let inner: *mut Inner<T> = *self.inner.get_mut();
loom::ptr_with_mut(&mut self.inner, |inner| {
let inner: *mut Inner<T> = *inner;

// If the state pointer has been initialized, deallocate it.
if !inner.is_null() {
unsafe {
drop(Arc::from_raw(inner));
// If the state pointer has been initialized, deallocate it.
if !inner.is_null() {
unsafe {
drop(Arc::from_raw(inner));
}
}
}
});
}
}

Expand Down Expand Up @@ -817,7 +843,10 @@ fn full_fence() {
}
}

#[cfg(not(loom))]
mod loom {
pub(crate) use core::cell;

#[cfg(not(feature = "portable-atomic"))]
pub(crate) use core::sync::atomic;

Expand All @@ -829,6 +858,32 @@ mod loom {

#[cfg(feature = "portable-atomic")]
pub(crate) use portable_atomic_util::Arc;

/// Equivalent to `loom::AtomicPtr::with_mut`
pub(crate) fn ptr_with_mut<T, R>(
ptr: &mut atomic::AtomicPtr<T>,
f: impl FnOnce(&mut *mut T) -> R,
) -> R {
f(ptr.get_mut())
}
}

#[cfg(loom)]
mod loom {
pub(crate) use loom::cell;
pub(crate) use loom::sync::Arc;

pub(crate) mod atomic {
pub(crate) use core::sync::atomic::compiler_fence;
pub(crate) use loom::sync::atomic::*;
}

pub(crate) fn ptr_with_mut<T, R>(
ptr: &mut atomic::AtomicPtr<T>,
f: impl FnOnce(&mut *mut T) -> R,
) -> R {
ptr.with_mut(f)
}
}

mod __sealed {
Expand Down
36 changes: 26 additions & 10 deletions src/linked_list/lock_free.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
//! Implementation of the linked list using lock-free primitives.

use crate::loom::atomic::{AtomicBool, AtomicPtr, AtomicUsize, Ordering};
use crate::loom::cell::{Cell, UnsafeCell};
use crate::notify::{GenericNotify, Internal, Notification};

use core::cell::{Cell, UnsafeCell};
#[cfg(not(loom))]
use core::hint::spin_loop;
#[cfg(loom)]
use loom::hint::spin_loop;

use core::cmp::Reverse;
use core::fmt;
use core::hint::spin_loop;
use core::iter;
use core::marker::PhantomData;
use core::mem::{self, MaybeUninit};
Expand Down Expand Up @@ -662,14 +666,16 @@ impl<T> Drop for Slots<T> {
fn drop(&mut self) {
// Free every bucket.
for (i, bucket) in self.buckets.iter_mut().enumerate() {
let bucket = *bucket.get_mut();
if bucket.is_null() {
continue;
}
crate::loom::ptr_with_mut(bucket, |bucket| {
let bucket = *bucket;
if bucket.is_null() {
return;
}

// Drop the bucket.
let size = bucket_index_to_size(i);
drop(unsafe { Box::from_raw(slice::from_raw_parts_mut(bucket, size)) });
// Drop the bucket.
let size = bucket_index_to_size(i);
drop(unsafe { Box::from_raw(slice::from_raw_parts_mut(bucket, size)) });
});
}
}
}
Expand Down Expand Up @@ -737,13 +743,23 @@ impl<T> Lock<T> {
let _drop = CallOnDrop(|| self.is_locked.store(false, Ordering::Release));

// SAFETY: We have exclusive access.
Some(f(unsafe { &mut *self.data.get() }))
Some(cell_with_mut(&self.data, |ptr| f(unsafe { &mut *ptr })))
} else {
None
}
}
}

#[cfg(not(loom))]
fn cell_with_mut<T, R>(cell: &UnsafeCell<T>, f: impl FnOnce(*mut T) -> R) -> R {
f(cell.get())
}

#[cfg(loom)]
fn cell_with_mut<T, R>(cell: &UnsafeCell<T>, f: impl FnOnce(*mut T) -> R) -> R {
cell.with_mut(f)
}

#[inline]
fn bucket_index_to_size(i: usize) -> usize {
1 << i
Expand Down
9 changes: 5 additions & 4 deletions src/linked_list/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Implementation of the linked list using standard library mutexes.

use crate::loom::atomic::{AtomicUsize, Ordering};
use crate::loom::cell::Cell;
use crate::notify::{GenericNotify, Internal, Notification};

use std::boxed::Box;
use std::cell::{Cell, UnsafeCell};
use std::fmt;
use std::mem;
use std::ops::{Deref, DerefMut};
Expand All @@ -26,7 +26,8 @@ pub(crate) struct Inner<T> {
list: Mutex<List<T>>,

/// A single cached list entry to avoid allocations on the fast path of the insertion.
cache: UnsafeCell<Entry<T>>,
// TODO: Add ability to use loom::cell::UnsafeCell
cache: std::cell::UnsafeCell<Entry<T>>,
}

impl<T> Inner<T> {
Expand All @@ -42,7 +43,7 @@ impl<T> Inner<T> {
notified: 0,
cache_used: false,
}),
cache: UnsafeCell::new(Entry {
cache: std::cell::UnsafeCell::new(Entry {
state: Cell::new(State::Created),
prev: Cell::new(None),
next: Cell::new(None),
Expand Down Expand Up @@ -403,7 +404,7 @@ impl<T> List<T> {
entry.as_ref().state.replace(State::Created)
} else {
// Deallocate the entry.
Box::from_raw(entry.as_ptr()).state.into_inner()
Box::from_raw(entry.as_ptr()).state.replace(State::Created)
};

// Update the counters.
Expand Down

0 comments on commit bae7aef

Please sign in to comment.