Skip to content

Commit

Permalink
Merge branch 'make-loom-optional'
Browse files Browse the repository at this point in the history
  • Loading branch information
faern committed Jun 10, 2024
2 parents f2846be + 54b401e commit d911a58
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 61 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ jobs:
- name: Build
run: cargo build

# Run `cargo test` with all combinations of features. Including loom correctness testing
- name: Test
run: cargo hack --feature-powerset test
run: cargo hack --feature-powerset --exclude-all-features --optional-deps loom test

# Run `cargo test` but with artificial delay injected into some code paths. This helps
# running through some hard-to-time code paths. loom testing is not included since it is not
# compatible nor make any sense together with sleeping.
- name: Test with artificial delay
shell: bash
run: RUSTFLAGS+="--cfg oneshot_test_delay" cargo hack --feature-powerset test

- name: Test with loom
shell: bash
run: RUSTFLAGS+="--cfg loom" LOOM_MAX_BRANCHES=100000 cargo hack --feature-powerset test --test sync --test loom
run: RUSTFLAGS+="--cfg oneshot_test_delay" cargo hack --feature-powerset --exclude-all-features test
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.


## [Unreleased]
### Changed
- Change how loom concurrency testing is triggered. Make it an optional dependency instead of
placed behind a custom `cfg(loom)`. This has the pros that it does not end up in the dependency
tree of `oneshot` (unless the optional feature `loom` is enabled), which makes this library
way smaller for downstream consumers. This has the downside that the crate now exposes a `loom`
feature. DOWNSTREAM USERS ARE NOT SUPPOSED TO EVER ENABLE THIS. No stability or semver
guarantees exist around the `loom` feature.


## [0.1.7] - 2024-05-24
Expand Down
9 changes: 4 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,17 @@ std = []
# Enables async receiving by implementing Future
async = []

[target.'cfg(loom)'.dependencies]
loom = { version = "0.7.2", features = ["futures"] }
[dependencies]
# Only used for internal correctness testing. Never enable this feature when using oneshot.
loom = { version = "0.7.2", features = ["futures"], optional = true }

[dev-dependencies]
criterion = "0.3"

[target.'cfg(not(loom))'.dev-dependencies]
tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "time"] }
async-std = { version = "1", features = ["attributes"] }

[lints.rust]
unexpected_cfgs = { level = "deny", check-cfg = ['cfg(loom)', 'cfg(oneshot_test_delay)'] }
unexpected_cfgs = { level = "deny", check-cfg = ['cfg(oneshot_test_delay)'] }

[[bench]]
name = "benches"
Expand Down
50 changes: 26 additions & 24 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
//! can receive both blocking and async.
//!
//! ```rust
//! # #[cfg(not(feature = "loom"))] {
//! use std::sync::mpsc;
//! use std::thread;
//! use std::time::Duration;
Expand Down Expand Up @@ -71,6 +72,7 @@
//! }
//! });
//! }
//! # }
//! ```
//!
//! # Sync vs async
Expand Down Expand Up @@ -114,7 +116,7 @@
#![deny(rust_2018_idioms)]
#![cfg_attr(not(feature = "std"), no_std)]

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
extern crate alloc;

use core::{
Expand All @@ -123,20 +125,20 @@ use core::{
ptr::{self, NonNull},
};

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
use core::{
cell::UnsafeCell,
sync::atomic::{fence, AtomicU8, Ordering::*},
};
#[cfg(loom)]
#[cfg(feature = "loom")]
use loom::{
cell::UnsafeCell,
sync::atomic::{fence, AtomicU8, Ordering::*},
};

#[cfg(all(feature = "async", not(loom)))]
#[cfg(all(feature = "async", not(feature = "loom")))]
use core::hint;
#[cfg(all(feature = "async", loom))]
#[cfg(all(feature = "async", feature = "loom"))]
use loom::hint;

#[cfg(feature = "async")]
Expand All @@ -149,10 +151,10 @@ use std::time::{Duration, Instant};

#[cfg(feature = "std")]
mod thread {
#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
pub use std::thread::{current, park, park_timeout, Thread};

#[cfg(loom)]
#[cfg(feature = "loom")]
pub use loom::thread::{current, park, Thread};

// loom does not support parking with a timeout. So we just
Expand All @@ -161,17 +163,17 @@ mod thread {
// One thing to note is that very short timeouts are needed
// when using loom, since otherwise the looping will cause
// an overflow in loom.
#[cfg(loom)]
#[cfg(feature = "loom")]
pub fn park_timeout(_timeout: std::time::Duration) {
loom::thread::yield_now()
}
}

#[cfg(loom)]
#[cfg(feature = "loom")]
mod loombox;
#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
use alloc::boxed::Box;
#[cfg(loom)]
#[cfg(feature = "loom")]
use loombox::Box;

mod errors;
Expand Down Expand Up @@ -484,7 +486,7 @@ impl<T> Receiver<T> {
// Conditionally add a delay here to help the tests trigger the edge cases where
// the sender manages to be dropped or send something before we are able to store
// our waker object in the channel.
#[cfg(oneshot_test_delay)]
#[cfg(all(oneshot_test_delay, not(feature = "loom")))]
std::thread::sleep(std::time::Duration::from_millis(10));

// Write our waker instance to the channel.
Expand Down Expand Up @@ -791,7 +793,7 @@ impl<T> Receiver<T> {
// Conditionally add a delay here to help the tests trigger the edge cases where
// the sender manages to be dropped or send something before we are able to store
// our waker object in the channel.
#[cfg(oneshot_test_delay)]
#[cfg(all(oneshot_test_delay, not(feature = "loom")))]
std::thread::sleep(std::time::Duration::from_millis(10));

// Write our waker instance to the channel.
Expand Down Expand Up @@ -1051,12 +1053,12 @@ impl<T> Channel<T> {

#[inline(always)]
unsafe fn message(&self) -> &MaybeUninit<T> {
#[cfg(loom)]
#[cfg(feature = "loom")]
{
self.message.with(|ptr| &*ptr)
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
{
&*self.message.get()
}
Expand All @@ -1067,12 +1069,12 @@ impl<T> Channel<T> {
where
F: FnOnce(&mut MaybeUninit<T>),
{
#[cfg(loom)]
#[cfg(feature = "loom")]
{
self.message.with_mut(|ptr| op(&mut *ptr))
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
{
op(&mut *self.message.get())
}
Expand All @@ -1084,12 +1086,12 @@ impl<T> Channel<T> {
where
F: FnOnce(&mut MaybeUninit<ReceiverWaker>),
{
#[cfg(loom)]
#[cfg(feature = "loom")]
{
self.waker.with_mut(|ptr| op(&mut *ptr))
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
{
op(&mut *self.waker.get())
}
Expand All @@ -1102,12 +1104,12 @@ impl<T> Channel<T> {

#[inline(always)]
unsafe fn take_message(&self) -> T {
#[cfg(loom)]
#[cfg(feature = "loom")]
{
self.message.with(|ptr| ptr::read(ptr)).assume_init()
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
{
ptr::read(self.message.get()).assume_init()
}
Expand All @@ -1126,12 +1128,12 @@ impl<T> Channel<T> {

#[inline(always)]
unsafe fn take_waker(&self) -> ReceiverWaker {
#[cfg(loom)]
#[cfg(feature = "loom")]
{
self.waker.with(|ptr| ptr::read(ptr)).assume_init()
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
{
ptr::read(self.waker.get()).assume_init()
}
Expand Down Expand Up @@ -1231,7 +1233,7 @@ impl ReceiverWaker {
}
}

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
#[test]
fn receiver_waker_size() {
let expected: usize = match (cfg!(feature = "std"), cfg!(feature = "async")) {
Expand Down
8 changes: 4 additions & 4 deletions src/loombox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,25 @@ impl<T: ?Sized> core::ops::DerefMut for Box<T> {

impl<T: ?Sized> borrow::Borrow<T> for Box<T> {
fn borrow(&self) -> &T {
&**self
self
}
}

impl<T: ?Sized> borrow::BorrowMut<T> for Box<T> {
fn borrow_mut(&mut self) -> &mut T {
&mut **self
self
}
}

impl<T: ?Sized> AsRef<T> for Box<T> {
fn as_ref(&self) -> &T {
&**self
self
}
}

impl<T: ?Sized> AsMut<T> for Box<T> {
fn as_mut(&mut self) -> &mut T {
&mut **self
self
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/async.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(all(feature = "async", not(loom)))]
#![cfg(all(feature = "async", not(feature = "loom")))]

use core::mem;
use core::time::Duration;
Expand Down
4 changes: 2 additions & 2 deletions tests/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

use core::{future, mem, pin, task};

#[cfg(loom)]
#[cfg(feature = "loom")]
pub use loom::sync::{Arc, Mutex};
#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
pub use std::sync::{Arc, Mutex};

mod helpers;
Expand Down
12 changes: 6 additions & 6 deletions tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@

extern crate alloc;

#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
use alloc::sync::Arc;
#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
use core::sync::atomic::{AtomicUsize, Ordering::SeqCst};
#[cfg(loom)]
#[cfg(feature = "loom")]
use loom::sync::{
atomic::{AtomicUsize, Ordering::SeqCst},
Arc,
};

#[cfg(loom)]
#[cfg(feature = "loom")]
pub mod waker;

pub fn maybe_loom_model(test: impl Fn() + Sync + Send + 'static) {
#[cfg(loom)]
#[cfg(feature = "loom")]
loom::model(test);
#[cfg(not(loom))]
#[cfg(not(feature = "loom"))]
test();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/loom.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(loom)]
#![cfg(feature = "loom")]

use oneshot::TryRecvError;

Expand Down
2 changes: 1 addition & 1 deletion tests/raw.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(not(loom))]
#![cfg(not(feature = "loom"))]

use oneshot::{channel, Receiver, Sender};

Expand Down
Loading

0 comments on commit d911a58

Please sign in to comment.