From 7069400c477c71609928024dc9a3416eb1336041 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Fri, 31 Oct 2025 14:49:45 -0400 Subject: [PATCH 1/3] revert combined nonpoison/poison tests for condvar Setup for writing different tests for the `nonpoison::Condvar` since it will have a different API. Signed-off-by: Connor Tsui --- library/std/tests/sync/condvar.rs | 472 ++++++++++++++---------------- 1 file changed, 226 insertions(+), 246 deletions(-) diff --git a/library/std/tests/sync/condvar.rs b/library/std/tests/sync/condvar.rs index 42b880e283afe..74328793caeae 100644 --- a/library/std/tests/sync/condvar.rs +++ b/library/std/tests/sync/condvar.rs @@ -17,256 +17,237 @@ nonpoison_and_poison_unwrap_test!( } ); +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: notify_one, - test_body: { - use locks::{Condvar, Mutex}; +fn notify_one() { + use std::sync::{Condvar, Mutex}; - let m = Arc::new(Mutex::new(())); - let m2 = m.clone(); - let c = Arc::new(Condvar::new()); - let c2 = c.clone(); + let m = Arc::new(Mutex::new(())); + let m2 = m.clone(); + let c = Arc::new(Condvar::new()); + let c2 = c.clone(); - let g = maybe_unwrap(m.lock()); - let _t = thread::spawn(move || { - let _g = maybe_unwrap(m2.lock()); - c2.notify_one(); - }); - let g = maybe_unwrap(c.wait(g)); - drop(g); - } -); + let g = m.lock().unwrap(); + let _t = thread::spawn(move || { + let _g = m2.lock().unwrap(); + c2.notify_one(); + }); -#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: notify_all, - test_body: { - use locks::{Condvar, Mutex}; - - const N: usize = 10; - - let data = Arc::new((Mutex::new(0), Condvar::new())); - let (tx, rx) = channel(); - for _ in 0..N { - let data = data.clone(); - let tx = tx.clone(); - thread::spawn(move || { - let &(ref lock, ref cond) = &*data; - let mut cnt = maybe_unwrap(lock.lock()); - *cnt += 1; - if *cnt == N { - tx.send(()).unwrap(); - } - while *cnt != 0 { - cnt = maybe_unwrap(cond.wait(cnt)); - } - tx.send(()).unwrap(); - }); - } - drop(tx); - - let &(ref lock, ref cond) = &*data; - rx.recv().unwrap(); - let mut cnt = maybe_unwrap(lock.lock()); - *cnt = 0; - cond.notify_all(); - drop(cnt); - - for _ in 0..N { - rx.recv().unwrap(); - } - } -); + let g = c.wait(g).unwrap(); + drop(g); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: test_mutex_arc_condvar, - test_body: { - use locks::{Condvar, Mutex}; - - struct Packet(Arc<(Mutex, Condvar)>); +fn notify_all() { + use std::sync::{Condvar, Mutex}; - let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); - let packet2 = Packet(packet.0.clone()); + const N: usize = 10; - let (tx, rx) = channel(); - - let _t = thread::spawn(move || { - // Wait until our parent has taken the lock. - rx.recv().unwrap(); - let &(ref lock, ref cvar) = &*packet2.0; - - // Set the data to `true` and wake up our parent. - let mut guard = maybe_unwrap(lock.lock()); - *guard = true; - cvar.notify_one(); + let data = Arc::new((Mutex::new(0), Condvar::new())); + let (tx, rx) = channel(); + for _ in 0..N { + let data = data.clone(); + let tx = tx.clone(); + thread::spawn(move || { + let &(ref lock, ref cond) = &*data; + let mut cnt = lock.lock().unwrap(); + *cnt += 1; + if *cnt == N { + tx.send(()).unwrap(); + } + while *cnt != 0 { + cnt = cond.wait(cnt).unwrap(); + } + tx.send(()).unwrap(); }); + } + drop(tx); - let &(ref lock, ref cvar) = &*packet.0; - let mut guard = maybe_unwrap(lock.lock()); - // Wake up our child. - tx.send(()).unwrap(); + let &(ref lock, ref cond) = &*data; + rx.recv().unwrap(); + let mut cnt = lock.lock().unwrap(); + *cnt = 0; + cond.notify_all(); + drop(cnt); - // Wait until our child has set the data to `true`. - assert!(!*guard); - while !*guard { - guard = maybe_unwrap(cvar.wait(guard)); - } + for _ in 0..N { + rx.recv().unwrap(); } -); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_while, - test_body: { - use locks::{Condvar, Mutex}; +fn test_mutex_arc_condvar() { + use std::sync::{Condvar, Mutex}; - let pair = Arc::new((Mutex::new(false), Condvar::new())); - let pair2 = pair.clone(); + struct Packet(Arc<(Mutex, Condvar)>); - // Inside of our lock, spawn a new thread, and then wait for it to start. - thread::spawn(move || { - let &(ref lock, ref cvar) = &*pair2; - let mut started = maybe_unwrap(lock.lock()); - *started = true; - // We notify the condvar that the value has changed. - cvar.notify_one(); - }); + let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); + let packet2 = Packet(packet.0.clone()); - // Wait for the thread to start up. - let &(ref lock, ref cvar) = &*pair; - let guard = cvar.wait_while(maybe_unwrap(lock.lock()), |started| !*started); - assert!(*maybe_unwrap(guard)); + let (tx, rx) = channel(); + + let _t = thread::spawn(move || { + // Wait until our parent has taken the lock. + rx.recv().unwrap(); + let &(ref lock, ref cvar) = &*packet2.0; + + // Set the data to `true` and wake up our parent. + let mut guard = lock.lock().unwrap(); + *guard = true; + cvar.notify_one(); + }); + + let &(ref lock, ref cvar) = &*packet.0; + let mut guard = lock.lock().unwrap(); + // Wake up our child. + tx.send(()).unwrap(); + + // Wait until our child has set the data to `true`. + assert!(!*guard); + while !*guard { + guard = cvar.wait(guard).unwrap(); } -); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_timeout_wait, - test_body: { - use locks::{Condvar, Mutex}; - - let m = Arc::new(Mutex::new(())); - let c = Arc::new(Condvar::new()); - - loop { - let g = maybe_unwrap(m.lock()); - let (_g, no_timeout) = maybe_unwrap(c.wait_timeout(g, Duration::from_millis(1))); - // spurious wakeups mean this isn't necessarily true - // so execute test again, if not timeout - if !no_timeout.timed_out() { - continue; - } - - break; +fn wait_while() { + use std::sync::{Condvar, Mutex}; + + let pair = Arc::new((Mutex::new(false), Condvar::new())); + let pair2 = pair.clone(); + + // Inside of our lock, spawn a new thread, and then wait for it to start. + thread::spawn(move || { + let &(ref lock, ref cvar) = &*pair2; + let mut started = lock.lock().unwrap(); + *started = true; + // We notify the condvar that the value has changed. + cvar.notify_one(); + }); + + // Wait for the thread to start up. + let &(ref lock, ref cvar) = &*pair; + let guard = cvar.wait_while(lock.lock().unwrap(), |started| !*started).unwrap(); + assert!(*guard); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn wait_timeout_wait() { + use std::sync::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); + + loop { + let g = m.lock().unwrap(); + let (_g, no_timeout) = c.wait_timeout(g, Duration::from_millis(1)).unwrap(); + // spurious wakeups mean this isn't necessarily true + // so execute test again, if not timeout + if !no_timeout.timed_out() { + continue; } + + break; } -); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_timeout_while_wait, - test_body: { - use locks::{Condvar, Mutex}; +fn wait_timeout_while_wait() { + use std::sync::{Condvar, Mutex}; - let m = Arc::new(Mutex::new(())); - let c = Arc::new(Condvar::new()); + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); - let g = maybe_unwrap(m.lock()); - let (_g, wait) = maybe_unwrap(c.wait_timeout_while(g, Duration::from_millis(1), |_| true)); - // no spurious wakeups. ensure it timed-out - assert!(wait.timed_out()); - } -); + let g = m.lock().unwrap(); + let (_g, wait) = c.wait_timeout_while(g, Duration::from_millis(1), |_| true).unwrap(); + // no spurious wakeups. ensure it timed-out + assert!(wait.timed_out()); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_timeout_while_instant_satisfy, - test_body: { - use locks::{Condvar, Mutex}; +fn wait_timeout_while_instant_satisfy() { + use std::sync::{Condvar, Mutex}; - let m = Arc::new(Mutex::new(())); - let c = Arc::new(Condvar::new()); + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); - let g = maybe_unwrap(m.lock()); - let (_g, wait) = - maybe_unwrap(c.wait_timeout_while(g, Duration::from_millis(0), |_| false)); - // ensure it didn't time-out even if we were not given any time. - assert!(!wait.timed_out()); - } -); + let g = m.lock().unwrap(); + let (_g, wait) = c.wait_timeout_while(g, Duration::from_millis(0), |_| false).unwrap(); + // ensure it didn't time-out even if we were not given any time. + assert!(!wait.timed_out()); +} +#[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_timeout_while_wake, - test_body: { - use locks::{Condvar, Mutex}; +fn wait_timeout_while_wake() { + use std::sync::{Condvar, Mutex}; + + let pair = Arc::new((Mutex::new(false), Condvar::new())); + let pair_copy = pair.clone(); + + let &(ref m, ref c) = &*pair; + let g = m.lock().unwrap(); + let _t = thread::spawn(move || { + let &(ref lock, ref cvar) = &*pair_copy; + let mut started = lock.lock().unwrap(); + thread::sleep(Duration::from_millis(1)); + *started = true; + cvar.notify_one(); + }); + + let (g2, wait) = c + .wait_timeout_while(g, Duration::from_millis(u64::MAX), |&mut notified| !notified) + .unwrap(); + // ensure it didn't time-out even if we were not given any time. + assert!(!wait.timed_out()); + assert!(*g2); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn wait_timeout_wake() { + use std::sync::{Condvar, Mutex}; - let pair = Arc::new((Mutex::new(false), Condvar::new())); - let pair_copy = pair.clone(); + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); - let &(ref m, ref c) = &*pair; - let g = maybe_unwrap(m.lock()); - let _t = thread::spawn(move || { - let &(ref lock, ref cvar) = &*pair_copy; - let mut started = maybe_unwrap(lock.lock()); - thread::sleep(Duration::from_millis(1)); - *started = true; - cvar.notify_one(); - }); - let (g2, wait) = maybe_unwrap(c.wait_timeout_while( - g, - Duration::from_millis(u64::MAX), - |&mut notified| !notified - )); - // ensure it didn't time-out even if we were not given any time. - assert!(!wait.timed_out()); - assert!(*g2); - } -); + loop { + let g = m.lock().unwrap(); -#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -nonpoison_and_poison_unwrap_test!( - name: wait_timeout_wake, - test_body: { - use locks::{Condvar, Mutex}; + let c2 = c.clone(); + let m2 = m.clone(); - let m = Arc::new(Mutex::new(())); - let c = Arc::new(Condvar::new()); + let notified = Arc::new(AtomicBool::new(false)); + let notified_copy = notified.clone(); - loop { - let g = maybe_unwrap(m.lock()); - - let c2 = c.clone(); - let m2 = m.clone(); - - let notified = Arc::new(AtomicBool::new(false)); - let notified_copy = notified.clone(); - - let t = thread::spawn(move || { - let _g = maybe_unwrap(m2.lock()); - thread::sleep(Duration::from_millis(1)); - notified_copy.store(true, Ordering::Relaxed); - c2.notify_one(); - }); - let (g, timeout_res) = - maybe_unwrap(c.wait_timeout(g, Duration::from_millis(u64::MAX))); - assert!(!timeout_res.timed_out()); - // spurious wakeups mean this isn't necessarily true - // so execute test again, if not notified - if !notified.load(Ordering::Relaxed) { - t.join().unwrap(); - continue; - } - drop(g); + let t = thread::spawn(move || { + let _g = m2.lock().unwrap(); + thread::sleep(Duration::from_millis(1)); + notified_copy.store(true, Ordering::Relaxed); + c2.notify_one(); + }); + let (g, timeout_res) = c.wait_timeout(g, Duration::from_millis(u64::MAX)).unwrap(); + assert!(!timeout_res.timed_out()); + // spurious wakeups mean this isn't necessarily true + // so execute test again, if not notified + if !notified.load(Ordering::Relaxed) { t.join().unwrap(); - - break; + continue; } + drop(g); + + t.join().unwrap(); + + break; } -); +} // Some platforms internally cast the timeout duration into nanoseconds. // If they fail to consider overflow during the conversion (I'm looking @@ -274,42 +255,41 @@ nonpoison_and_poison_unwrap_test!( // timeout for durations that are slightly longer than u64::MAX nanoseconds. // `std` should guard against this by clamping the timeout. // See #37440 for context. -nonpoison_and_poison_unwrap_test!( - name: timeout_nanoseconds, - test_body: { - use locks::Mutex; - use locks::Condvar; +#[test] +fn timeout_nanoseconds() { + use std::sync::{Condvar, Mutex}; + + let sent = Mutex::new(false); + let cond = Condvar::new(); + + thread::scope(|s| { + s.spawn(|| { + // Sleep so that the other thread has a chance to encounter the + // timeout. + thread::sleep(Duration::from_secs(2)); + *sent.lock().unwrap() = true; + cond.notify_all(); + }); - let sent = Mutex::new(false); - let cond = Condvar::new(); - - thread::scope(|s| { - s.spawn(|| { - // Sleep so that the other thread has a chance to encounter the - // timeout. - thread::sleep(Duration::from_secs(2)); - maybe_unwrap(sent.set(true)); - cond.notify_all(); - }); - - let mut guard = maybe_unwrap(sent.lock()); - // Loop until `sent` is set by the thread to guard against spurious - // wakeups. If the `wait_timeout` happens just before the signal by - // the other thread, such a spurious wakeup might prevent the - // miscalculated timeout from occurring, but this is basically just - // a smoke test anyway. - loop { - if *guard { - break; - } - - // If there is internal overflow, this call will return almost - // immediately, before the other thread has reached the `notify_all`, - // and indicate a timeout. - let (g, res) = maybe_unwrap(cond.wait_timeout(guard, Duration::from_secs(u64::MAX.div_ceil(1_000_000_000)))); - assert!(!res.timed_out()); - guard = g; + let mut guard = sent.lock().unwrap(); + // Loop until `sent` is set by the thread to guard against spurious + // wakeups. If the `wait_timeout` happens just before the signal by + // the other thread, such a spurious wakeup might prevent the + // miscalculated timeout from occurring, but this is basically just + // a smoke test anyway. + loop { + if *guard { + break; } - }) - } -); + + // If there is internal overflow, this call will return almost + // immediately, before the other thread has reached the `notify_all`, + // and indicate a timeout. + let (g, res) = cond + .wait_timeout(guard, Duration::from_secs(u64::MAX.div_ceil(1_000_000_000))) + .unwrap(); + assert!(!res.timed_out()); + guard = g; + } + }) +} From 3d5a40809c8322de933205a4d581059dc8adc3f3 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Fri, 31 Oct 2025 15:12:46 -0400 Subject: [PATCH 2/3] update `nonpoison::Condvar` to take guards by reference Since non-poisoning `Condvar` take non-poisoing `Mutex`es when `wait`ing, we do not need to take by ownership since a poison error cannot occur while we wait. Signed-off-by: Connor Tsui --- library/std/src/sync/barrier.rs | 2 +- library/std/src/sync/nonpoison/condvar.rs | 71 +++-- library/std/tests/sync/condvar.rs | 309 ++++++++++++++++++++-- 3 files changed, 324 insertions(+), 58 deletions(-) diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index 8988126bd90c0..c2c18889dde7d 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -125,7 +125,7 @@ impl Barrier { let local_gen = lock.generation_id; lock.count += 1; if lock.count < self.num_threads { - let _guard = self.cvar.wait_while(lock, |state| local_gen == state.generation_id); + self.cvar.wait_while(&mut lock, |state| local_gen == state.generation_id); BarrierWaitResult(false) } else { lock.count = 0; diff --git a/library/std/src/sync/nonpoison/condvar.rs b/library/std/src/sync/nonpoison/condvar.rs index 994fc6816a0d0..d2b251d7c44c1 100644 --- a/library/std/src/sync/nonpoison/condvar.rs +++ b/library/std/src/sync/nonpoison/condvar.rs @@ -1,4 +1,5 @@ use crate::fmt; +use crate::ops::DerefMut; use crate::sync::WaitTimeoutResult; use crate::sync::nonpoison::{MutexGuard, mutex}; use crate::sys::sync as sys; @@ -38,7 +39,7 @@ use crate::time::{Duration, Instant}; /// let (lock, cvar) = &*pair; /// let mut started = lock.lock(); /// while !*started { -/// started = cvar.wait(started); +/// cvar.wait(&mut started); /// } /// ``` /// @@ -115,16 +116,15 @@ impl Condvar { /// let mut started = lock.lock(); /// // As long as the value inside the `Mutex` is `false`, we wait. /// while !*started { - /// started = cvar.wait(started); + /// cvar.wait(&mut started); /// } /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] - pub fn wait<'a, T>(&self, guard: MutexGuard<'a, T>) -> MutexGuard<'a, T> { + pub fn wait(&self, guard: &mut MutexGuard<'_, T>) { unsafe { - let lock = mutex::guard_lock(&guard); + let lock = mutex::guard_lock(guard); self.inner.wait(lock); } - guard } /// Blocks the current thread until the provided condition becomes false. @@ -167,21 +167,17 @@ impl Condvar { /// // Wait for the thread to start up. /// let (lock, cvar) = &*pair; /// // As long as the value inside the `Mutex` is `true`, we wait. - /// let _guard = cvar.wait_while(lock.lock(), |pending| { *pending }); + /// let mut guard = lock.lock(); + /// cvar.wait_while(&mut guard, |pending| { *pending }); /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] - pub fn wait_while<'a, T, F>( - &self, - mut guard: MutexGuard<'a, T>, - mut condition: F, - ) -> MutexGuard<'a, T> + pub fn wait_while(&self, guard: &mut MutexGuard<'_, T>, mut condition: F) where F: FnMut(&mut T) -> bool, { - while condition(&mut *guard) { - guard = self.wait(guard); + while condition(guard.deref_mut()) { + self.wait(guard); } - guard } /// Waits on this condition variable for a notification, timing out after a @@ -206,7 +202,7 @@ impl Condvar { /// The returned [`WaitTimeoutResult`] value indicates if the timeout is /// known to have elapsed. /// - /// Like [`wait`], the lock specified will be re-acquired when this function + /// Like [`wait`], the lock specified will have been re-acquired when this function /// returns, regardless of whether the timeout elapsed or not. /// /// [`wait`]: Self::wait @@ -239,9 +235,8 @@ impl Condvar { /// let mut started = lock.lock(); /// // as long as the value inside the `Mutex` is `false`, we wait /// loop { - /// let result = cvar.wait_timeout(started, Duration::from_millis(10)); + /// let result = cvar.wait_timeout(&mut started, Duration::from_millis(10)); /// // 10 milliseconds have passed, or maybe the value changed! - /// started = result.0; /// if *started == true { /// // We received the notification and the value has been updated, we can leave. /// break @@ -249,16 +244,16 @@ impl Condvar { /// } /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] - pub fn wait_timeout<'a, T>( + pub fn wait_timeout( &self, - guard: MutexGuard<'a, T>, + guard: &mut MutexGuard<'_, T>, dur: Duration, - ) -> (MutexGuard<'a, T>, WaitTimeoutResult) { + ) -> WaitTimeoutResult { let success = unsafe { - let lock = mutex::guard_lock(&guard); + let lock = mutex::guard_lock(guard); self.inner.wait_timeout(lock, dur) }; - (guard, WaitTimeoutResult(!success)) + WaitTimeoutResult(!success) } /// Waits on this condition variable for a notification, timing out after a @@ -277,7 +272,7 @@ impl Condvar { /// The returned [`WaitTimeoutResult`] value indicates if the timeout is /// known to have elapsed without the condition being met. /// - /// Like [`wait_while`], the lock specified will be re-acquired when this + /// Like [`wait_while`], the lock specified will have been re-acquired when this /// function returns, regardless of whether the timeout elapsed or not. /// /// [`wait_while`]: Self::wait_while @@ -307,37 +302,39 @@ impl Condvar { /// /// // wait for the thread to start up /// let (lock, cvar) = &*pair; + /// let mut guard = lock.lock(); /// let result = cvar.wait_timeout_while( - /// lock.lock(), + /// &mut guard, /// Duration::from_millis(100), /// |&mut pending| pending, /// ); - /// if result.1.timed_out() { + /// if result.timed_out() { /// // timed-out without the condition ever evaluating to false. /// } - /// // access the locked mutex via result.0 + /// // access the locked mutex via guard /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] - pub fn wait_timeout_while<'a, T, F>( + pub fn wait_timeout_while( &self, - mut guard: MutexGuard<'a, T>, + guard: &mut MutexGuard<'_, T>, dur: Duration, mut condition: F, - ) -> (MutexGuard<'a, T>, WaitTimeoutResult) + ) -> WaitTimeoutResult where F: FnMut(&mut T) -> bool, { let start = Instant::now(); - loop { - if !condition(&mut *guard) { - return (guard, WaitTimeoutResult(false)); - } + + while condition(guard.deref_mut()) { let timeout = match dur.checked_sub(start.elapsed()) { Some(timeout) => timeout, - None => return (guard, WaitTimeoutResult(true)), + None => return WaitTimeoutResult(true), }; - guard = self.wait_timeout(guard, timeout).0; + + self.wait_timeout(guard, timeout); } + + WaitTimeoutResult(false) } /// Wakes up one blocked thread on this condvar. @@ -378,7 +375,7 @@ impl Condvar { /// let mut started = lock.lock(); /// // As long as the value inside the `Mutex` is `false`, we wait. /// while !*started { - /// started = cvar.wait(started); + /// cvar.wait(&mut started); /// } /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] @@ -422,7 +419,7 @@ impl Condvar { /// let mut started = lock.lock(); /// // As long as the value inside the `Mutex` is `false`, we wait. /// while !*started { - /// started = cvar.wait(started); + /// cvar.wait(&mut started); /// } /// ``` #[unstable(feature = "nonpoison_condvar", issue = "134645")] diff --git a/library/std/tests/sync/condvar.rs b/library/std/tests/sync/condvar.rs index 74328793caeae..a52e0a00caf48 100644 --- a/library/std/tests/sync/condvar.rs +++ b/library/std/tests/sync/condvar.rs @@ -19,8 +19,8 @@ nonpoison_and_poison_unwrap_test!( #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn notify_one() { - use std::sync::{Condvar, Mutex}; +fn poison_notify_one() { + use std::sync::poison::{Condvar, Mutex}; let m = Arc::new(Mutex::new(())); let m2 = m.clone(); @@ -39,8 +39,28 @@ fn notify_one() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn notify_all() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_notify_one() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let m2 = m.clone(); + let c = Arc::new(Condvar::new()); + let c2 = c.clone(); + + let mut g = m.lock(); + let _t = thread::spawn(move || { + let _g = m2.lock(); + c2.notify_one(); + }); + + c.wait(&mut g); + drop(g); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_notify_all() { + use std::sync::poison::{Condvar, Mutex}; const N: usize = 10; @@ -78,8 +98,47 @@ fn notify_all() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn test_mutex_arc_condvar() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_notify_all() { + use std::sync::nonpoison::{Condvar, Mutex}; + + const N: usize = 10; + + let data = Arc::new((Mutex::new(0), Condvar::new())); + let (tx, rx) = channel(); + for _ in 0..N { + let data = data.clone(); + let tx = tx.clone(); + thread::spawn(move || { + let &(ref lock, ref cond) = &*data; + let mut cnt = lock.lock(); + *cnt += 1; + if *cnt == N { + tx.send(()).unwrap(); + } + while *cnt != 0 { + cond.wait(&mut cnt); + } + tx.send(()).unwrap(); + }); + } + drop(tx); + + let &(ref lock, ref cond) = &*data; + rx.recv().unwrap(); + let mut cnt = lock.lock(); + *cnt = 0; + cond.notify_all(); + drop(cnt); + + for _ in 0..N { + rx.recv().unwrap(); + } +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_test_mutex_arc_condvar() { + use std::sync::poison::{Condvar, Mutex}; struct Packet(Arc<(Mutex, Condvar)>); @@ -113,8 +172,43 @@ fn test_mutex_arc_condvar() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_while() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_test_mutex_arc_condvar() { + use std::sync::nonpoison::{Condvar, Mutex}; + + struct Packet(Arc<(Mutex, Condvar)>); + + let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); + let packet2 = Packet(packet.0.clone()); + + let (tx, rx) = channel(); + + let _t = thread::spawn(move || { + // Wait until our parent has taken the lock. + rx.recv().unwrap(); + let &(ref lock, ref cvar) = &*packet2.0; + + // Set the data to `true` and wake up our parent. + let mut guard = lock.lock(); + *guard = true; + cvar.notify_one(); + }); + + let &(ref lock, ref cvar) = &*packet.0; + let mut guard = lock.lock(); + // Wake up our child. + tx.send(()).unwrap(); + + // Wait until our child has set the data to `true`. + assert!(!*guard); + while !*guard { + cvar.wait(&mut guard); + } +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_while() { + use std::sync::poison::{Condvar, Mutex}; let pair = Arc::new((Mutex::new(false), Condvar::new())); let pair2 = pair.clone(); @@ -136,8 +230,32 @@ fn wait_while() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_timeout_wait() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_wait_while() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let pair = Arc::new((Mutex::new(false), Condvar::new())); + let pair2 = pair.clone(); + + // Inside of our lock, spawn a new thread, and then wait for it to start. + thread::spawn(move || { + let &(ref lock, ref cvar) = &*pair2; + let mut started = lock.lock(); + *started = true; + // We notify the condvar that the value has changed. + cvar.notify_one(); + }); + + // Wait for the thread to start up. + let &(ref lock, ref cvar) = &*pair; + let mut guard = lock.lock(); + cvar.wait_while(&mut guard, |started| !*started); + assert!(*guard); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_timeout_wait() { + use std::sync::poison::{Condvar, Mutex}; let m = Arc::new(Mutex::new(())); let c = Arc::new(Condvar::new()); @@ -157,8 +275,29 @@ fn wait_timeout_wait() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_timeout_while_wait() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_wait_timeout_wait() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); + + loop { + let mut g = m.lock(); + let no_timeout = c.wait_timeout(&mut g, Duration::from_millis(1)); + // spurious wakeups mean this isn't necessarily true + // so execute test again, if not timeout + if !no_timeout.timed_out() { + continue; + } + + break; + } +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_timeout_while_wait() { + use std::sync::poison::{Condvar, Mutex}; let m = Arc::new(Mutex::new(())); let c = Arc::new(Condvar::new()); @@ -171,8 +310,22 @@ fn wait_timeout_while_wait() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_timeout_while_instant_satisfy() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_wait_timeout_while_wait() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); + + let mut g = m.lock(); + let wait = c.wait_timeout_while(&mut g, Duration::from_millis(1), |_| true); + // no spurious wakeups. ensure it timed-out + assert!(wait.timed_out()); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_timeout_while_instant_satisfy() { + use std::sync::poison::{Condvar, Mutex}; let m = Arc::new(Mutex::new(())); let c = Arc::new(Condvar::new()); @@ -185,8 +338,22 @@ fn wait_timeout_while_instant_satisfy() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_timeout_while_wake() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_wait_timeout_while_instant_satisfy() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); + + let mut g = m.lock(); + let wait = c.wait_timeout_while(&mut g, Duration::from_millis(0), |_| false); + // ensure it didn't time-out even if we were not given any time. + assert!(!wait.timed_out()); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_timeout_while_wake() { + use std::sync::poison::{Condvar, Mutex}; let pair = Arc::new((Mutex::new(false), Condvar::new())); let pair_copy = pair.clone(); @@ -211,8 +378,33 @@ fn wait_timeout_while_wake() { #[test] #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. -fn wait_timeout_wake() { - use std::sync::{Condvar, Mutex}; +fn nonpoison_wait_timeout_while_wake() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let pair = Arc::new((Mutex::new(false), Condvar::new())); + let pair_copy = pair.clone(); + + let &(ref m, ref c) = &*pair; + let mut g = m.lock(); + let _t = thread::spawn(move || { + let &(ref lock, ref cvar) = &*pair_copy; + let mut started = lock.lock(); + thread::sleep(Duration::from_millis(1)); + *started = true; + cvar.notify_one(); + }); + + let wait = + c.wait_timeout_while(&mut g, Duration::from_millis(u64::MAX), |&mut notified| !notified); + // ensure it didn't time-out even if we were not given any time. + assert!(!wait.timed_out()); + assert!(*g); +} + +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn poison_wait_timeout_wake() { + use std::sync::poison::{Condvar, Mutex}; let m = Arc::new(Mutex::new(())); let c = Arc::new(Condvar::new()); @@ -249,6 +441,46 @@ fn wait_timeout_wake() { } } +#[test] +#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads. +fn nonpoison_wait_timeout_wake() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let m = Arc::new(Mutex::new(())); + let c = Arc::new(Condvar::new()); + + loop { + let mut g = m.lock(); + + let c2 = c.clone(); + let m2 = m.clone(); + + let notified = Arc::new(AtomicBool::new(false)); + let notified_copy = notified.clone(); + + let t = thread::spawn(move || { + let _g = m2.lock(); + thread::sleep(Duration::from_millis(1)); + notified_copy.store(true, Ordering::Relaxed); + c2.notify_one(); + }); + + let timeout_res = c.wait_timeout(&mut g, Duration::from_millis(u64::MAX)); + assert!(!timeout_res.timed_out()); + // spurious wakeups mean this isn't necessarily true + // so execute test again, if not notified + if !notified.load(Ordering::Relaxed) { + t.join().unwrap(); + continue; + } + drop(g); + + t.join().unwrap(); + + break; + } +} + // Some platforms internally cast the timeout duration into nanoseconds. // If they fail to consider overflow during the conversion (I'm looking // at you, macOS), `wait_timeout` will return immediately and indicate a @@ -256,8 +488,8 @@ fn wait_timeout_wake() { // `std` should guard against this by clamping the timeout. // See #37440 for context. #[test] -fn timeout_nanoseconds() { - use std::sync::{Condvar, Mutex}; +fn poison_timeout_nanoseconds() { + use std::sync::poison::{Condvar, Mutex}; let sent = Mutex::new(false); let cond = Condvar::new(); @@ -293,3 +525,40 @@ fn timeout_nanoseconds() { } }) } + +#[test] +fn nonpoison_timeout_nanoseconds() { + use std::sync::nonpoison::{Condvar, Mutex}; + + let sent = Mutex::new(false); + let cond = Condvar::new(); + + thread::scope(|s| { + s.spawn(|| { + // Sleep so that the other thread has a chance to encounter the + // timeout. + thread::sleep(Duration::from_secs(2)); + sent.set(true); + cond.notify_all(); + }); + + let mut guard = sent.lock(); + // Loop until `sent` is set by the thread to guard against spurious + // wakeups. If the `wait_timeout` happens just before the signal by + // the other thread, such a spurious wakeup might prevent the + // miscalculated timeout from occurring, but this is basically just + // a smoke test anyway. + loop { + if *guard { + break; + } + + // If there is internal overflow, this call will return almost + // immediately, before the other thread has reached the `notify_all`, + // and indicate a timeout. + let res = cond + .wait_timeout(&mut guard, Duration::from_secs(u64::MAX.div_ceil(1_000_000_000))); + assert!(!res.timed_out()); + } + }) +} From c1153b08ff87d09ab043bf1878cfb3f1cf55c8ec Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Fri, 31 Oct 2025 15:14:21 -0400 Subject: [PATCH 3/3] move condvar test from mutex to condvar test file Signed-off-by: Connor Tsui --- library/std/tests/sync/condvar.rs | 34 +++++++++++++++++++++++++++++++ library/std/tests/sync/mutex.rs | 34 +------------------------------ 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/library/std/tests/sync/condvar.rs b/library/std/tests/sync/condvar.rs index a52e0a00caf48..e5a7ad8f9b331 100644 --- a/library/std/tests/sync/condvar.rs +++ b/library/std/tests/sync/condvar.rs @@ -562,3 +562,37 @@ fn nonpoison_timeout_nanoseconds() { } }) } + +#[test] +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +fn test_arc_condvar_poison() { + use std::sync::poison::{Condvar, Mutex}; + + struct Packet(Arc<(Mutex, Condvar)>); + + let packet = Packet(Arc::new((Mutex::new(1), Condvar::new()))); + let packet2 = Packet(packet.0.clone()); + let (tx, rx) = channel(); + + let _t = thread::spawn(move || -> () { + rx.recv().unwrap(); + let &(ref lock, ref cvar) = &*packet2.0; + let _g = lock.lock().unwrap(); + cvar.notify_one(); + // Parent should fail when it wakes up. + panic!(); + }); + + let &(ref lock, ref cvar) = &*packet.0; + let mut lock = lock.lock().unwrap(); + tx.send(()).unwrap(); + while *lock == 1 { + match cvar.wait(lock) { + Ok(l) => { + lock = l; + assert_eq!(*lock, 1); + } + Err(..) => break, + } + } +} diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index ff6aef717936f..75a6bf64607ef 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -3,7 +3,7 @@ use std::ops::FnMut; use std::panic::{self, AssertUnwindSafe}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::channel; -use std::sync::{Arc, Condvar, MappedMutexGuard, Mutex, MutexGuard, TryLockError}; +use std::sync::{Arc, MappedMutexGuard, Mutex, MutexGuard, TryLockError}; use std::{hint, mem, thread}; //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -423,38 +423,6 @@ fn test_replace_poison() { inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); } -#[test] -#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] -fn test_arc_condvar_poison() { - struct Packet(Arc<(Mutex, Condvar)>); - - let packet = Packet(Arc::new((Mutex::new(1), Condvar::new()))); - let packet2 = Packet(packet.0.clone()); - let (tx, rx) = channel(); - - let _t = thread::spawn(move || -> () { - rx.recv().unwrap(); - let &(ref lock, ref cvar) = &*packet2.0; - let _g = lock.lock().unwrap(); - cvar.notify_one(); - // Parent should fail when it wakes up. - panic!(); - }); - - let &(ref lock, ref cvar) = &*packet.0; - let mut lock = lock.lock().unwrap(); - tx.send(()).unwrap(); - while *lock == 1 { - match cvar.wait(lock) { - Ok(l) => { - lock = l; - assert_eq!(*lock, 1); - } - Err(..) => break, - } - } -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_mutex_arc_poison() {