From 23b90f3e8dca2ba9620e88b9399aecf3ca944b05 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sat, 13 Apr 2024 21:42:23 -0700 Subject: [PATCH] tests: Add further testing Co-authored-by: Jacob Rothstein Signed-off-by: John Nunley --- .github/workflows/ci.yml | 53 +++++++++++++++--------- Cargo.toml | 9 +++-- benches/bench.rs | 11 ++--- examples/mutex.rs | 4 +- src/lib.rs | 61 ++++++++++++++-------------- src/notify.rs | 12 +----- tests/notify.rs | 66 +++++++++++++++++++++++++++++- tests/park.rs | 87 +++++++++++++++++++++++++++++++++++++++- 8 files changed, 225 insertions(+), 78 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 689989d..01a2d02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,7 @@ on: env: CARGO_INCREMENTAL: 0 + CARGO_NET_GIT_FETCH_WITH_CLI: true CARGO_NET_RETRY: 10 CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 @@ -26,34 +27,44 @@ defaults: jobs: test: - runs-on: ${{ matrix.os }} + runs-on: ubuntu-latest strategy: fail-fast: false matrix: - os: [ubuntu-latest] - rust: [nightly, beta, stable] + include: + - rust: stable + - rust: beta + - rust: nightly + - rust: nightly + target: i686-unknown-linux-gnu steps: - uses: actions/checkout@v4 - name: Install Rust run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }} + - name: Install cross-compilation tools + uses: taiki-e/setup-cross-toolchain-action@v1 + with: + target: ${{ matrix.target }} + if: matrix.target != '' - run: cargo build --all --all-features --all-targets + - run: cargo test --all + - run: cargo test --all --release + - run: cargo test --no-default-features --tests + - run: cargo test --no-default-features --tests --release + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - run: rustup target add thumbv7m-none-eabi - name: Run cargo check (without dev-dependencies to catch missing feature flags) - if: startsWith(matrix.rust, 'nightly') - run: cargo check -Z features=dev_dep - - run: cargo test + run: cargo hack build --all --no-dev-deps msrv: runs-on: ubuntu-latest - strategy: - matrix: - # When updating this, the reminder to update the minimum supported - # Rust version in Cargo.toml. - rust: ['1.39'] steps: - uses: actions/checkout@v4 - - name: Install Rust - run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }} - - run: cargo build + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - run: cargo hack build --all --rust-version + - run: cargo hack build --all --no-default-features --rust-version clippy: runs-on: ubuntu-latest @@ -61,7 +72,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust run: rustup update stable - - run: cargo clippy --all-features --all-targets + - run: cargo clippy --all --all-features --all-targets fmt: runs-on: ubuntu-latest @@ -77,10 +88,12 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust run: rustup toolchain install nightly --component miri && rustup default nightly - - run: cargo miri test - env: - MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation - RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout + - run: | + echo "MIRIFLAGS=-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation" >>"${GITHUB_ENV}" + echo "RUSTFLAGS=${RUSTFLAGS} -Z randomize-layout" >>"${GITHUB_ENV}" + - run: cargo miri test --all + - run: cargo miri test --no-default-features --tests + - run: cargo miri test --no-default-features --features portable-atomic --tests security_audit: permissions: @@ -89,7 +102,7 @@ jobs: issues: write runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 # https://github.com/rustsec/audit-check/issues/2 - uses: rustsec/audit-check@master with: diff --git a/Cargo.toml b/Cargo.toml index 1662626..a2e393f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,11 +2,11 @@ name = "event-listener" # When publishing a new version: # - Update CHANGELOG.md -# - Create "v2.x.y" git tag +# - Create "v5.x.y" git tag version = "5.3.0" -authors = ["Stjepan Glavina "] +authors = ["Stjepan Glavina ", "John Nunley "] edition = "2021" -rust-version = "1.56" +rust-version = "1.63" description = "Notify async tasks or threads" license = "Apache-2.0 OR MIT" repository = "https://github.com/smol-rs/event-listener" @@ -21,7 +21,7 @@ portable-atomic = ["portable-atomic-crate", "portable-atomic-util"] [dependencies] portable-atomic-crate = { version = "1.6.0", package = "portable-atomic", optional = true } -portable-atomic-util = { version = "0.1.5", optional = true } +portable-atomic-util = { version = "0.1.5", optional = true, features = ["alloc"] } [dev-dependencies] futures = { version = "0.3", default-features = false, features = ["std"] } @@ -32,6 +32,7 @@ waker-fn = "1" [[bench]] name = "bench" harness = false +required-features = ["std"] [lib] bench = false \ No newline at end of file diff --git a/benches/bench.rs b/benches/bench.rs index c120ee9..283f6d2 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -1,21 +1,18 @@ use criterion::{criterion_group, criterion_main, Criterion}; use event_listener::{Event, Listener}; +use std::iter; const COUNT: usize = 8000; fn bench_events(c: &mut Criterion) { c.bench_function("notify_and_wait", |b| { let ev = Event::new(); + let mut handles = Vec::with_capacity(COUNT); b.iter(|| { - let mut handles = Vec::with_capacity(COUNT); - - for _ in 0..COUNT { - handles.push(ev.listen()); - } - + handles.extend(iter::repeat_with(|| ev.listen()).take(COUNT)); ev.notify(COUNT); - for handle in handles { + for handle in handles.drain(..) { handle.wait(); } }); diff --git a/examples/mutex.rs b/examples/mutex.rs index 4f68e05..4fc9e74 100644 --- a/examples/mutex.rs +++ b/examples/mutex.rs @@ -90,9 +90,7 @@ impl Mutex { } Some(l) => { // Wait until a notification is received. - if l.wait_deadline(deadline).is_none() { - return None; - } + l.wait_deadline(deadline)?; } } } diff --git a/src/lib.rs b/src/lib.rs index f517db5..959b323 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,8 +75,8 @@ )] #![warn(missing_docs, missing_debug_implementations, rust_2018_idioms)] -use loom::atomic::{self, AtomicPtr, AtomicUsize, Ordering}; -use loom::Arc; +use crate::loom::atomic::{self, AtomicPtr, AtomicUsize, Ordering}; +use crate::loom::Arc; use notify::{GenericNotify, Internal, NotificationPrivate}; use std::cell::{Cell, UnsafeCell}; @@ -91,7 +91,6 @@ use std::sync::{Mutex, MutexGuard, TryLockError}; use std::task::{Context, Poll, Waker}; use std::thread::{self, Thread}; use std::time::{Duration, Instant}; -use std::usize; mod notify; pub use notify::{IntoNotification, Notification}; @@ -385,24 +384,6 @@ impl Event { pub fn notify_additional_relaxed(&self, n: usize) -> usize { self.notify(n.additional().relaxed()) } -} - -impl Event { - /// Creates a new [`Event`] with a tag. - /// - /// # Examples - /// - /// ``` - /// use event_listener::Event; - /// - /// let event = Event::::with_tag(); - /// ``` - #[inline] - pub const fn with_tag() -> Event { - Event { - inner: AtomicPtr::new(ptr::null_mut()), - } - } /// Return the listener count by acquiring a lock. /// @@ -430,15 +411,32 @@ impl Event { /// drop(listener2); /// assert_eq!(event.total_listeners(), 0); /// ``` - #[cfg(feature = "std")] #[inline] pub fn total_listeners(&self) -> usize { if let Some(inner) = self.try_inner() { - inner.total_listeners() + inner.lock().len } else { 0 } } +} + +impl Event { + /// Creates a new [`Event`] with a tag. + /// + /// # Examples + /// + /// ``` + /// use event_listener::Event; + /// + /// let event = Event::::with_tag(); + /// ``` + #[inline] + pub const fn with_tag() -> Event { + Event { + inner: AtomicPtr::new(ptr::null_mut()), + } + } /// Returns a guard listening for a notification. /// @@ -504,7 +502,7 @@ impl Event { if let Some(inner) = self.try_inner() { let limit = if notify.is_additional(Internal::new()) { // Notify if there is at least one unnotified listener. - std::usize::MAX + usize::MAX } else { // Notify if there is at least one unnotified listener and the number of notified // listeners is less than `n`. @@ -535,7 +533,8 @@ impl Event { /// ``` #[inline] pub fn is_notified(&self) -> bool { - self.try_inner().map_or(false, |inner| inner.notified.load(Ordering::Acquire) > 0) + self.try_inner() + .map_or(false, |inner| inner.notified.load(Ordering::Acquire) > 0) } /// Returns a reference to the inner state if it was initialized. @@ -614,11 +613,13 @@ impl Drop for Event { impl fmt::Debug for Event { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let inner = match self.try_inner() { - None => return f - .debug_tuple("Event") - .field(&format_args!("")) - .finish(), - Some(inner) => inner + None => { + return f + .debug_tuple("Event") + .field(&format_args!("")) + .finish() + } + Some(inner) => inner, }; let guard = match inner.list.try_lock() { diff --git a/src/notify.rs b/src/notify.rs index c88a609..8b708d2 100644 --- a/src/notify.rs +++ b/src/notify.rs @@ -1,6 +1,5 @@ //! The `Notification` trait for specifying notification. -#[cfg(feature = "std")] use core::fmt; use crate::loom::atomic::{self, Ordering}; @@ -161,7 +160,6 @@ where } /// Use a tag to notify listeners. -#[cfg(feature = "std")] #[derive(Debug, Clone)] #[doc(hidden)] pub struct Tag { @@ -169,7 +167,6 @@ pub struct Tag { inner: N, } -#[cfg(feature = "std")] impl Tag { /// Create a new `Tag` with the given tag and notification. fn new(tag: T, inner: N) -> Self @@ -180,7 +177,6 @@ impl Tag { } } -#[cfg(feature = "std")] impl NotificationPrivate for Tag where N: Notification + ?Sized, @@ -206,14 +202,12 @@ where } /// Use a function to generate a tag to notify listeners. -#[cfg(feature = "std")] #[doc(hidden)] pub struct TagWith { tag: F, inner: N, } -#[cfg(feature = "std")] impl fmt::Debug for TagWith { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("TagWith") @@ -223,7 +217,6 @@ impl fmt::Debug for TagWith { } } -#[cfg(feature = "std")] impl TagWith { /// Create a new `TagFn` with the given tag function and notification. fn new(tag: F, inner: N) -> Self { @@ -231,7 +224,6 @@ impl TagWith { } } -#[cfg(feature = "std")] impl NotificationPrivate for TagWith where N: Notification + ?Sized, @@ -490,7 +482,6 @@ pub trait IntoNotification: __private::Sealed { /// assert_eq!(listener1.wait(), true); /// assert_eq!(listener2.wait(), false); /// ``` - #[cfg(feature = "std")] fn tag(self, tag: T) -> Tag where Self: Sized + IntoNotification, @@ -524,7 +515,6 @@ pub trait IntoNotification: __private::Sealed { /// assert_eq!(listener1.wait(), true); /// assert_eq!(listener2.wait(), false); /// ``` - #[cfg(feature = "std")] fn tag_with(self, tag: F) -> TagWith where Self: Sized + IntoNotification, @@ -568,7 +558,7 @@ impl_for_numeric_types! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } /// Equivalent to `atomic::fence(Ordering::SeqCst)`, but in some cases faster. #[inline] pub(super) fn full_fence() { - #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), not(miri), not(loom)))] + #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), not(miri)))] { use core::{arch::asm, cell::UnsafeCell}; // HACK(stjepang): On x86 architectures there are two different ways of executing diff --git a/tests/notify.rs b/tests/notify.rs index 5431c24..c984cd1 100644 --- a/tests/notify.rs +++ b/tests/notify.rs @@ -2,9 +2,8 @@ use std::future::Future; use std::pin::Pin; use std::sync::{Arc, Mutex}; use std::task::Context; -use std::usize; -use event_listener::{Event, EventListener}; +use event_listener::{Event, EventListener, Listener}; use waker_fn::waker_fn; fn is_notified(listener: &mut EventListener) -> bool { @@ -14,6 +13,20 @@ fn is_notified(listener: &mut EventListener) -> bool { .is_ready() } +#[test] +fn debug() { + let event = Event::new(); + let fmt = format!("{:?}", &event); + assert!(fmt.contains("Event")); + + let listener = event.listen(); + let fmt = format!("{:?}", &listener); + assert!(fmt.contains("EventListener")); + + let fmt = format!("{:?}", &event); + assert!(fmt.contains("Event")); +} + #[test] fn notify() { let event = Event::new(); @@ -192,6 +205,55 @@ fn drop_non_notified() { assert!(!is_notified(&mut l2)); } +#[test] +fn discard() { + let event = Event::default(); + + let l1 = event.listen(); + assert!(!l1.discard()); + + let l1 = event.listen(); + event.notify(1); + assert!(l1.discard()); + + let l1 = event.listen(); + event.notify_additional(1); + assert!(l1.discard()); + + let mut l1 = event.listen(); + event.notify(1); + assert!(is_notified(&mut l1)); + assert!(!l1.discard()); +} + +#[test] +fn same_event() { + let e1 = Event::new(); + let e2 = Event::new(); + + let l1 = e1.listen(); + let l2 = e1.listen(); + let l3 = e2.listen(); + + assert!(l1.listens_to(&e1)); + assert!(!l1.listens_to(&e2)); + assert!(l1.same_event(&l2)); + assert!(!l1.same_event(&l3)); +} + +#[test] +#[should_panic = "cannot poll a completed `EventListener` future"] +fn poll_twice() { + let event = Event::new(); + let mut l1 = event.listen(); + event.notify(1); + + assert!(is_notified(&mut l1)); + + // Panic here. + is_notified(&mut l1); +} + #[test] fn notify_all_fair() { let event = Event::new(); diff --git a/tests/park.rs b/tests/park.rs index ae03322..48796be 100644 --- a/tests/park.rs +++ b/tests/park.rs @@ -1,8 +1,35 @@ //! Test the wait() family of methods. -use event_listener::{Event, IntoNotification, Listener}; +use event_listener::{Event, EventListener, IntoNotification, Listener}; + +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; +use std::task::Context; +use std::thread; use std::time::{Duration, Instant}; +use waker_fn::waker_fn; + +fn is_notified(listener: &mut EventListener) -> bool { + let waker = waker_fn(|| ()); + Pin::new(listener) + .poll(&mut Context::from_waker(&waker)) + .is_ready() +} + +#[test] +fn total_listeners() { + let event = Event::new(); + assert_eq!(event.total_listeners(), 0); + + let listener = event.listen(); + assert_eq!(event.total_listeners(), 1); + + drop(listener); + assert_eq!(event.total_listeners(), 0); +} + #[test] fn wait() { let event = Event::new(); @@ -30,6 +57,18 @@ fn wait_timeout() { assert_eq!(listener.wait_timeout(Duration::from_millis(50)), Some(())); } +#[test] +fn wait_deadline() { + let event = Event::new(); + let listener = event.listen(); + + assert_eq!(event.notify(1), 1); + assert_eq!( + listener.wait_deadline(Instant::now() + Duration::from_millis(50)), + Some(()) + ); +} + #[test] fn wait_timeout_expiry() { let event = Event::new(); @@ -39,3 +78,49 @@ fn wait_timeout_expiry() { assert_eq!(listener.wait_timeout(Duration::from_millis(200)), None); assert!(Instant::now().duration_since(start) >= Duration::from_millis(200)); } + +#[test] +fn unpark() { + let event = Arc::new(Event::new()); + let listener = event.listen(); + + thread::spawn({ + let event = event.clone(); + move || { + thread::sleep(Duration::from_millis(100)); + event.notify(1); + } + }); + + listener.wait(); +} + +#[test] +fn unpark_timeout() { + let event = Arc::new(Event::new()); + let listener = event.listen(); + + thread::spawn({ + let event = event.clone(); + move || { + thread::sleep(Duration::from_millis(100)); + event.notify(1); + } + }); + + let x = listener.wait_timeout(Duration::from_millis(200)); + assert!(x.is_some()); +} + +#[test] +#[should_panic = "cannot wait twice on an `EventListener`"] +fn wait_twice() { + let event = Event::new(); + let mut listener = event.listen(); + event.notify(1); + + assert!(is_notified(&mut listener)); + + // Panic here. + listener.wait(); +}