From 26066b2c34eb630b7852fdb8573b1ae93f824ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Tue, 11 Jun 2024 08:26:40 +0200 Subject: [PATCH] Put loom behind cfg(oneshot_loom) again --- .github/workflows/build-and-test.yml | 12 +++++-- CHANGELOG.md | 2 +- Cargo.toml | 8 +++-- src/lib.rs | 48 ++++++++++++++-------------- tests/async.rs | 2 +- tests/future.rs | 4 +-- tests/helpers/mod.rs | 12 +++---- tests/loom.rs | 2 +- tests/raw.rs | 2 +- tests/sync.rs | 22 ++++++------- 10 files changed, 62 insertions(+), 52 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index be5c6c9..fd892a9 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -40,10 +40,10 @@ jobs: shell: bash run: cargo build --locked - # Run `cargo test` with all combinations of features. Including loom correctness testing + # Run through all tests with all combinations of features - name: Test shell: bash - run: LOOM_MAX_BRANCHES=100000 cargo hack --feature-powerset --exclude-all-features --optional-deps loom test + run: cargo hack --feature-powerset --exclude-all-features 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 @@ -52,6 +52,14 @@ jobs: shell: bash run: RUSTFLAGS+="--cfg oneshot_test_delay" cargo hack --feature-powerset --exclude-all-features test + # Compile the library against loom to do correctness testing. + # `--features loom` must be given so that only feature powerset combinations including loom are tested. + - name: Test with loom + shell: bash + run: | + RUSTFLAGS+="--cfg oneshot_loom" LOOM_MAX_BRANCHES=100000 \ + cargo hack --feature-powerset --exclude-all-features --features loom test + # Check that the documentation builds and has no warnings - name: Build documentation shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 35cd79c..75058f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ 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 + placed behind a custom `cfg(oneshot_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 diff --git a/Cargo.toml b/Cargo.toml index 149f358..b02a85a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,8 +26,10 @@ std = [] # Enables async receiving by implementing Future async = [] -[dependencies] -# Only used for internal correctness testing. Never enable this feature when using oneshot. +# Only used for internal correctness testing. +# Downstream users of oneshot should never enable this feature. Enabling it does nothing. +# To compile oneshot built against loom one must *also* set RUSTFLAGS="--cfg oneshot_loom" +[target.'cfg(oneshot_loom)'.dependencies] loom = { version = "0.7.2", features = ["futures"], optional = true } [dev-dependencies] @@ -36,7 +38,7 @@ 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(oneshot_test_delay)'] } +unexpected_cfgs = { level = "deny", check-cfg = ['cfg(oneshot_loom)', 'cfg(oneshot_test_delay)'] } [[bench]] name = "benches" diff --git a/src/lib.rs b/src/lib.rs index 3194b59..b095341 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,7 +116,7 @@ #![deny(rust_2018_idioms)] #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] extern crate alloc; use core::{ @@ -125,20 +125,20 @@ use core::{ ptr::{self, NonNull}, }; -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] use core::{ cell::UnsafeCell, sync::atomic::{fence, AtomicU8, Ordering::*}, }; -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] use loom::{ cell::UnsafeCell, sync::atomic::{fence, AtomicU8, Ordering::*}, }; -#[cfg(all(feature = "async", not(feature = "loom")))] +#[cfg(all(feature = "async", not(oneshot_loom)))] use core::hint; -#[cfg(all(feature = "async", feature = "loom"))] +#[cfg(all(feature = "async", oneshot_loom))] use loom::hint; #[cfg(feature = "async")] @@ -151,10 +151,10 @@ use std::time::{Duration, Instant}; #[cfg(feature = "std")] mod thread { - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] pub use std::thread::{current, park, park_timeout, Thread}; - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] pub use loom::thread::{current, park, Thread}; // loom does not support parking with a timeout. So we just @@ -163,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(feature = "loom")] + #[cfg(oneshot_loom)] pub fn park_timeout(_timeout: std::time::Duration) { loom::thread::yield_now() } } -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] mod loombox; -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] use alloc::boxed::Box; -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] use loombox::Box; mod errors; @@ -488,7 +488,7 @@ impl Receiver { // 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(all(oneshot_test_delay, not(feature = "loom")))] + #[cfg(all(oneshot_test_delay, not(oneshot_loom)))] std::thread::sleep(std::time::Duration::from_millis(10)); // Write our waker instance to the channel. @@ -795,7 +795,7 @@ impl Receiver { // 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(all(oneshot_test_delay, not(feature = "loom")))] + #[cfg(all(oneshot_test_delay, not(oneshot_loom)))] std::thread::sleep(std::time::Duration::from_millis(10)); // Write our waker instance to the channel. @@ -1055,12 +1055,12 @@ impl Channel { #[inline(always)] unsafe fn message(&self) -> &MaybeUninit { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] { self.message.with(|ptr| &*ptr) } - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] { &*self.message.get() } @@ -1071,12 +1071,12 @@ impl Channel { where F: FnOnce(&mut MaybeUninit), { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] { self.message.with_mut(|ptr| op(&mut *ptr)) } - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] { op(&mut *self.message.get()) } @@ -1088,12 +1088,12 @@ impl Channel { where F: FnOnce(&mut MaybeUninit), { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] { self.waker.with_mut(|ptr| op(&mut *ptr)) } - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] { op(&mut *self.waker.get()) } @@ -1106,12 +1106,12 @@ impl Channel { #[inline(always)] unsafe fn take_message(&self) -> T { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] { self.message.with(|ptr| ptr::read(ptr)).assume_init() } - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] { ptr::read(self.message.get()).assume_init() } @@ -1130,12 +1130,12 @@ impl Channel { #[inline(always)] unsafe fn take_waker(&self) -> ReceiverWaker { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] { self.waker.with(|ptr| ptr::read(ptr)).assume_init() } - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] { ptr::read(self.waker.get()).assume_init() } @@ -1235,7 +1235,7 @@ impl ReceiverWaker { } } -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] #[test] fn receiver_waker_size() { let expected: usize = match (cfg!(feature = "std"), cfg!(feature = "async")) { diff --git a/tests/async.rs b/tests/async.rs index 6df8e02..6c0464d 100644 --- a/tests/async.rs +++ b/tests/async.rs @@ -1,4 +1,4 @@ -#![cfg(all(feature = "async", not(feature = "loom")))] +#![cfg(all(feature = "async", not(oneshot_loom)))] use core::mem; use core::time::Duration; diff --git a/tests/future.rs b/tests/future.rs index 49bbffc..373e890 100644 --- a/tests/future.rs +++ b/tests/future.rs @@ -2,9 +2,9 @@ use core::{future, mem, pin, task}; -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] pub use loom::sync::{Arc, Mutex}; -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] pub use std::sync::{Arc, Mutex}; mod helpers; diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 72ba670..c39b32e 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -2,23 +2,23 @@ extern crate alloc; -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] use alloc::sync::Arc; -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] use core::sync::atomic::{AtomicUsize, Ordering::SeqCst}; -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] use loom::sync::{ atomic::{AtomicUsize, Ordering::SeqCst}, Arc, }; -#[cfg(feature = "loom")] +#[cfg(oneshot_loom)] pub mod waker; pub fn maybe_loom_model(test: impl Fn() + Sync + Send + 'static) { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] loom::model(test); - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] test(); } diff --git a/tests/loom.rs b/tests/loom.rs index 02aec8e..4ce5f26 100644 --- a/tests/loom.rs +++ b/tests/loom.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "loom")] +#![cfg(oneshot_loom)] use oneshot::TryRecvError; diff --git a/tests/raw.rs b/tests/raw.rs index d0add53..285ae9c 100644 --- a/tests/raw.rs +++ b/tests/raw.rs @@ -1,4 +1,4 @@ -#![cfg(not(feature = "loom"))] +#![cfg(not(oneshot_loom))] use oneshot::{channel, Receiver, Sender}; diff --git a/tests/sync.rs b/tests/sync.rs index 45173b7..ae705e1 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -8,12 +8,12 @@ use std::time::{Duration, Instant}; #[cfg(feature = "std")] mod thread { - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] pub use loom::thread::spawn; - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] pub use std::thread::{sleep, spawn}; - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] pub fn sleep(_timeout: core::time::Duration) { loom::thread::yield_now() } @@ -59,7 +59,7 @@ fn send_before_recv() { // FIXME: This test does not work with loom. There is something that happens after the // channel object becomes larger than ~500 bytes and that makes an atomic read from the state // result in "signal: 10, SIGBUS: access to undefined memory" - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] maybe_loom_model(|| { let (sender, receiver) = oneshot::channel::<[u8; 4096]>(); assert!(sender.send([0b10101010; 4096]).is_ok()); @@ -246,16 +246,16 @@ fn recv_deadline_and_timeout_no_time() { } // This test doesn't give meaningful results when run with oneshot_test_delay and loom -#[cfg(all(feature = "std", not(all(oneshot_test_delay, feature = "loom"))))] +#[cfg(all(feature = "std", not(all(oneshot_test_delay, oneshot_loom))))] #[test] fn recv_deadline_time_should_elapse() { maybe_loom_model(|| { let (_sender, receiver) = oneshot::channel::(); let start = Instant::now(); - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] let timeout = Duration::from_millis(100); - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] let timeout = Duration::from_millis(1); assert_eq!( receiver.recv_deadline(start + timeout), @@ -266,16 +266,16 @@ fn recv_deadline_time_should_elapse() { }) } -#[cfg(all(feature = "std", not(all(oneshot_test_delay, feature = "loom"))))] +#[cfg(all(feature = "std", not(all(oneshot_test_delay, oneshot_loom))))] #[test] fn recv_timeout_time_should_elapse() { maybe_loom_model(|| { let (_sender, receiver) = oneshot::channel::(); let start = Instant::now(); - #[cfg(not(feature = "loom"))] + #[cfg(not(oneshot_loom))] let timeout = Duration::from_millis(100); - #[cfg(feature = "loom")] + #[cfg(oneshot_loom)] let timeout = Duration::from_millis(1); assert_eq!( @@ -287,7 +287,7 @@ fn recv_timeout_time_should_elapse() { }) } -#[cfg(not(feature = "loom"))] +#[cfg(not(oneshot_loom))] #[test] fn non_send_type_can_be_used_on_same_thread() { use std::ptr;