Skip to content

Commit

Permalink
feat(cordyceps): unsafe MpscQueue const constructor (#163)
Browse files Browse the repository at this point in the history
In order to permit constructing a `cordyceps::MpscQueue` in a `static`,
this branch adds a new `const fn new_with_static_stub` to `MpscQueue`.
This takes an `&'static T` as the stub node. Because this would
potentially allow using the same stub node in multiple queues, this
function is unfortunately unsafe.

This branch also bumps the nightly version for `mycelium`, because
`cordyceps` and `mycelium-util` now require Rust 1.61.0, for trait
bounds in `const fn`s. The nightly bump required some changes to the
Miri tests for cordyceps, as Miri now seems to consume more memory than
it did previously.

Co-authored-by: Eliza Weisman <[email protected]>
  • Loading branch information
jamesmunns and hawkw authored May 25, 2022
1 parent bae38c7 commit 7a3cede
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 25 deletions.
1 change: 1 addition & 0 deletions cordyceps/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ repository = "https://github.com/hawkw/mycelium"
readme = "README.md"
keywords = ["intrusive", "no_std", "list", "queue", "lock-free"]
categories = ["data-structures", "no-std"]
rust-version = "1.61.0"


# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
Expand Down
43 changes: 32 additions & 11 deletions cordyceps/src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,19 +775,40 @@ mod tests {
Remove(usize),
}

use core::ops::Range;
use proptest::collection::vec;
use proptest::num::usize::ANY;

/// Miri uses a significant amount of time and memory, meaning that
/// running 256 property tests (the default test-pass count) * (0..100)
/// vec elements (the default proptest vec length strategy) causes the
/// CI running to OOM (I think). In local testing, this required up
/// to 11GiB of resident memory with the default strategy, at the
/// time of this change.
///
/// In the future, it may be desirable to have an "override" feature
/// to use a larger test case set for more exhaustive local miri testing,
/// where the time and memory limitations are less restrictive than in CI.
#[cfg(miri)]
const FUZZ_RANGE: Range<usize> = 0..10;

/// The default range for proptest's vec strategy is 0..100.
#[cfg(not(miri))]
const FUZZ_RANGE: Range<usize> = 0..100;

proptest::proptest! {
#[test]
fn fuzz_linked_list(ops: Vec<usize>) {

let ops = ops
.iter()
.map(|i| match i % 3 {
0 => Op::Push,
1 => Op::Pop,
2 => Op::Remove(i / 3),
_ => unreachable!(),
})
.collect::<Vec<_>>();
fn fuzz_linked_list(ops in vec(ANY, FUZZ_RANGE)) {

let ops = ops
.iter()
.map(|i| match i % 3 {
0 => Op::Push,
1 => Op::Pop,
2 => Op::Remove(i / 3),
_ => unreachable!(),
})
.collect::<Vec<_>>();

let _trace = trace_init();
let _span = tracing::info_span!("fuzz").entered();
Expand Down
5 changes: 5 additions & 0 deletions cordyceps/src/loom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ mod inner {
Track { value }
}

#[inline(always)]
pub const fn new_const(value: T) -> Track<T> {
Track { value }
}

/// Get a reference to the value
#[inline(always)]
pub fn get_ref(&self) -> &T {
Expand Down
168 changes: 159 additions & 9 deletions cordyceps/src/mpsc_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,10 @@ pub struct MpscQueue<T: Linked<Links<T>>> {
/// new consumer.
has_consumer: CachePadded<AtomicBool>,

/// If the stub node is in a `static`, we cannot drop it when the
/// queue is dropped.
stub_is_static: bool,

stub: NonNull<T>,
}

Expand Down Expand Up @@ -404,7 +408,7 @@ impl<T: Linked<Links<T>>> MpscQueue<T> {
pub fn new_with_stub(stub: T::Handle) -> Self {
let stub = T::into_ptr(stub);

// // In debug mode, set the stub flag for consistency checking.
// In debug mode, set the stub flag for consistency checking.
#[cfg(debug_assertions)]
unsafe {
links(stub).is_stub.store(true, Release);
Expand All @@ -415,10 +419,101 @@ impl<T: Linked<Links<T>>> MpscQueue<T> {
head: CachePadded(AtomicPtr::new(ptr)),
tail: CachePadded(UnsafeCell::new(ptr)),
has_consumer: CachePadded(AtomicBool::new(false)),
stub_is_static: false,
stub,
}
}

/// Create an MpscQueue with a static "stub" entity
///
/// This is primarily used for creating an MpscQueue as a `static` variable.
///
/// # Usage notes
///
/// Unlike [`MpscQueue::new`] or [`MpscQueue::new_with_stub`], the `stub` item will NOT be
/// dropped when the `MpscQueue` is dropped. This is fine if you are
/// ALSO statically creating the `stub`, however if it is necessary to
/// recover that memory after the MpscQueue has been dropped, that will
/// need to be done by the user manually.
///
/// # Safety
///
/// The "stub" provided must ONLY EVER be used for a single MpscQueue. Re-using
/// the stub for multiple queues may lead to undefined behavior.
///
/// ## Example usage
///
/// ```rust
/// # use cordyceps::{
/// # Linked,
/// # mpsc_queue::{self, MpscQueue},
/// # };
/// # use std::{pin::Pin, ptr::NonNull, thread, sync::Arc};
/// #
/// #
///
/// // This is our same `Entry` from the parent examples. It has implemented
/// // the `Links` trait as above.
/// #[repr(C)]
/// #[derive(Debug, Default)]
/// struct Entry {
/// links: mpsc_queue::Links<Entry>,
/// val: i32,
/// }
///
/// #
/// # unsafe impl Linked<mpsc_queue::Links<Entry>> for Entry {
/// # type Handle = Pin<Box<Self>>;
/// #
/// # fn into_ptr(handle: Pin<Box<Entry>>) -> NonNull<Entry> {
/// # unsafe { NonNull::from(Box::leak(Pin::into_inner_unchecked(handle))) }
/// # }
/// #
/// # unsafe fn from_ptr(ptr: NonNull<Entry>) -> Pin<Box<Entry>> {
/// # Pin::new_unchecked(Box::from_raw(ptr.as_ptr()))
/// # }
/// #
/// # unsafe fn links(target: NonNull<Entry>) -> NonNull<mpsc_queue::Links<Entry>> {
/// # target.cast()
/// # }
/// # }
/// #
/// # impl Entry {
/// # fn new(val: i32) -> Self {
/// # Self {
/// # val,
/// # ..Self::default()
/// # }
/// # }
/// # }
///
///
/// static MPSC: MpscQueue<Entry> = {
/// static STUB_ENTRY: Entry = Entry {
/// links: mpsc_queue::Links::<Entry>::new_stub(),
/// val: 0
/// };
///
/// // SAFETY: The stub may not be used by another MPSC queue.
/// // Here, this is ensured because the `STUB_ENTRY` static is defined
/// // inside of the initializer for the `MPSC` static, so it cannot be referenced
/// // elsewhere.
/// unsafe { MpscQueue::new_with_static_stub(&STUB_ENTRY) }
/// };
/// ```
///
#[cfg(not(loom))]
pub const unsafe fn new_with_static_stub(stub: &'static T) -> Self {
let ptr = stub as *const T as *mut T;
Self {
head: CachePadded(AtomicPtr::new(ptr)),
tail: CachePadded(UnsafeCell::new(ptr)),
has_consumer: CachePadded(AtomicBool::new(false)),
stub_is_static: true,
stub: NonNull::new_unchecked(ptr),
}
}

/// Enqueue a new element at the end of the queue.
///
/// This takes ownership of a [`Handle`] that owns the element, and
Expand Down Expand Up @@ -707,7 +802,11 @@ impl<T: Linked<Links<T>>> Drop for MpscQueue<T> {
}

unsafe {
drop(T::from_ptr(self.stub));
// If the stub is static, don't drop it. It lives 5eva
// (that's one more than 4eva)
if !self.stub_is_static {
drop(T::from_ptr(self.stub));
}
}
}
}
Expand Down Expand Up @@ -867,6 +966,16 @@ impl<T> Links<T> {
}
}

#[cfg(not(loom))]
pub const fn new_stub() -> Self {
Self {
next: AtomicPtr::new(ptr::null_mut()),
_unpin: PhantomPinned,
#[cfg(debug_assertions)]
is_stub: AtomicBool::new(true),
}
}

#[cfg(loom)]
pub fn new() -> Self {
Self {
Expand All @@ -877,6 +986,16 @@ impl<T> Links<T> {
}
}

#[cfg(loom)]
pub fn new_stub() -> Self {
Self {
next: AtomicPtr::new(ptr::null_mut()),
_unpin: PhantomPinned,
#[cfg(debug_assertions)]
is_stub: AtomicBool::new(true),
}
}

#[cfg(debug_assertions)]
fn is_stub(&self) -> bool {
self.is_stub.load(Acquire)
Expand Down Expand Up @@ -1187,7 +1306,7 @@ mod tests {
use super::*;
use test_util::*;

use std::println;
use std::{ops::Deref, println, sync::Arc, thread};

#[test]
fn dequeue_empty() {
Expand Down Expand Up @@ -1237,17 +1356,39 @@ mod tests {

#[test]
fn basically_works() {
use std::{sync::Arc, thread};

const THREADS: i32 = if_miri(3, 8);
const MSGS: i32 = if_miri(10, 1000);

let stub = entry(666);
let q = MpscQueue::<Entry>::new_with_stub(stub);

assert_eq!(q.dequeue(), None);
let q = Arc::new(q);
test_basically_works(q);
}

#[test]
fn basically_works_all_const() {
static STUB_ENTRY: Entry = const_stub_entry(666);
static MPSC: MpscQueue<Entry> =
unsafe { MpscQueue::<Entry>::new_with_static_stub(&STUB_ENTRY) };
test_basically_works(&MPSC);
}

#[test]
fn basically_works_mixed_const() {
static STUB_ENTRY: Entry = const_stub_entry(666);
let q = unsafe { MpscQueue::<Entry>::new_with_static_stub(&STUB_ENTRY) };

let q = Arc::new(q);
test_basically_works(q)
}

fn test_basically_works<Q>(q: Q)
where
Q: Deref<Target = MpscQueue<Entry>> + Clone,
Q: Send + 'static,
{
const THREADS: i32 = if_miri(3, 8);
const MSGS: i32 = if_miri(10, 1000);

assert_eq!(q.dequeue(), None);

let threads: Vec<_> = (0..THREADS)
.map(|thread| {
Expand Down Expand Up @@ -1347,6 +1488,15 @@ mod test_util {
}
}

#[cfg(not(loom))]
pub(super) const fn const_stub_entry(val: i32) -> Entry {
Entry {
links: Links::new_stub(),
val,
_track: alloc::Track::new_const(()),
}
}

pub(super) fn entry(val: i32) -> Pin<Box<Entry>> {
Box::pin(Entry {
links: Links::new(),
Expand Down
4 changes: 2 additions & 2 deletions rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
[toolchain]
channel = "nightly-2022-02-15"
channel = "nightly-2022-05-24"
components = [
"clippy",
"rustfmt",
"rust-src",
"llvm-tools-preview",
"miri"
]
profile = "minimal"
profile = "minimal"
1 change: 1 addition & 0 deletions util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name = "mycelium-util"
version = "0.1.0"
authors = ["Eliza Weisman <[email protected]>"]
edition = "2018"
rust-version = "1.61.0"

# See more keys and their definitions at
# https://doc.rust-lang.org/cargo/reference/manifest.html
Expand Down
3 changes: 0 additions & 3 deletions util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
//! A "standard library" for programming in the Mycelium kernel and related
//! libraries.
#![cfg_attr(target_os = "none", no_std)]
#![feature(
const_fn_trait_bound // To allow trait bounds on const fn constructors.
)]
#![allow(unused_unsafe)]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg, doc_cfg_hide))]

Expand Down

0 comments on commit 7a3cede

Please sign in to comment.