From a8c2d4fc3d29496aa0a3563ec9d44f6222597fe3 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 00:57:46 +0200 Subject: [PATCH 01/12] Move boxing and mutex checking logic of condvar into sys_common. --- library/std/src/sync/condvar.rs | 44 ++------------ library/std/src/sys_common/condvar.rs | 64 +++++++++------------ library/std/src/sys_common/condvar/check.rs | 23 ++++++++ library/std/src/sys_common/mutex.rs | 2 +- 4 files changed, 55 insertions(+), 78 deletions(-) create mode 100644 library/std/src/sys_common/condvar/check.rs diff --git a/library/std/src/sync/condvar.rs b/library/std/src/sync/condvar.rs index 1376d8ebe8f4a..ffc1e57f4e03f 100644 --- a/library/std/src/sync/condvar.rs +++ b/library/std/src/sync/condvar.rs @@ -2,10 +2,8 @@ mod tests; use crate::fmt; -use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sync::{mutex, MutexGuard, PoisonError}; use crate::sys_common::condvar as sys; -use crate::sys_common::mutex as sys_mutex; use crate::sys_common::poison::{self, LockResult}; use crate::time::{Duration, Instant}; @@ -109,8 +107,7 @@ impl WaitTimeoutResult { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub struct Condvar { - inner: Box, - mutex: AtomicUsize, + inner: sys::Condvar, } impl Condvar { @@ -126,11 +123,7 @@ impl Condvar { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn new() -> Condvar { - let mut c = Condvar { inner: box sys::Condvar::new(), mutex: AtomicUsize::new(0) }; - unsafe { - c.inner.init(); - } - c + Condvar { inner: sys::Condvar::new() } } /// Blocks the current thread until this condition variable receives a @@ -192,7 +185,6 @@ impl Condvar { pub fn wait<'a, T>(&self, guard: MutexGuard<'a, T>) -> LockResult> { let poisoned = unsafe { let lock = mutex::guard_lock(&guard); - self.verify(lock); self.inner.wait(lock); mutex::guard_poison(&guard).get() }; @@ -389,7 +381,6 @@ impl Condvar { ) -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> { let (poisoned, result) = unsafe { let lock = mutex::guard_lock(&guard); - self.verify(lock); let success = self.inner.wait_timeout(lock, dur); (mutex::guard_poison(&guard).get(), WaitTimeoutResult(!success)) }; @@ -510,7 +501,7 @@ impl Condvar { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn notify_one(&self) { - unsafe { self.inner.notify_one() } + self.inner.notify_one() } /// Wakes up all blocked threads on this condvar. @@ -550,27 +541,7 @@ impl Condvar { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn notify_all(&self) { - unsafe { self.inner.notify_all() } - } - - fn verify(&self, mutex: &sys_mutex::MovableMutex) { - let addr = mutex.raw() as *const _ as usize; - match self.mutex.compare_and_swap(0, addr, Ordering::SeqCst) { - // If we got out 0, then we have successfully bound the mutex to - // this cvar. - 0 => {} - - // If we get out a value that's the same as `addr`, then someone - // already beat us to the punch. - n if n == addr => {} - - // Anything else and we're using more than one mutex on this cvar, - // which is currently disallowed. - _ => panic!( - "attempted to use a condition variable with two \ - mutexes" - ), - } + self.inner.notify_all() } } @@ -588,10 +559,3 @@ impl Default for Condvar { Condvar::new() } } - -#[stable(feature = "rust1", since = "1.0.0")] -impl Drop for Condvar { - fn drop(&mut self) { - unsafe { self.inner.destroy() } - } -} diff --git a/library/std/src/sys_common/condvar.rs b/library/std/src/sys_common/condvar.rs index a48d301f8127b..c65f1f81509d2 100644 --- a/library/std/src/sys_common/condvar.rs +++ b/library/std/src/sys_common/condvar.rs @@ -1,72 +1,62 @@ use crate::sys::condvar as imp; use crate::sys_common::mutex::MovableMutex; use crate::time::Duration; +use check::CondvarCheck; + +mod check; /// An OS-based condition variable. -/// -/// This structure is the lowest layer possible on top of the OS-provided -/// condition variables. It is consequently entirely unsafe to use. It is -/// recommended to use the safer types at the top level of this crate instead of -/// this type. -pub struct Condvar(imp::Condvar); +pub struct Condvar { + inner: Box, + check: CondvarCheck, +} impl Condvar { /// Creates a new condition variable for use. - /// - /// Behavior is undefined if the condition variable is moved after it is - /// first used with any of the functions below. - pub const fn new() -> Condvar { - Condvar(imp::Condvar::new()) - } - - /// Prepares the condition variable for use. - /// - /// This should be called once the condition variable is at a stable memory - /// address. - #[inline] - pub unsafe fn init(&mut self) { - self.0.init() + pub fn new() -> Self { + let mut c = box imp::Condvar::new(); + unsafe { c.init() }; + Self { inner: c, check: CondvarCheck::new() } } /// Signals one waiter on this condition variable to wake up. #[inline] - pub unsafe fn notify_one(&self) { - self.0.notify_one() + pub fn notify_one(&self) { + unsafe { self.inner.notify_one() }; } /// Awakens all current waiters on this condition variable. #[inline] - pub unsafe fn notify_all(&self) { - self.0.notify_all() + pub fn notify_all(&self) { + unsafe { self.inner.notify_all() }; } /// Waits for a signal on the specified mutex. /// /// Behavior is undefined if the mutex is not locked by the current thread. - /// Behavior is also undefined if more than one mutex is used concurrently - /// on this condition variable. + /// + /// May panic if used with more than one mutex. #[inline] pub unsafe fn wait(&self, mutex: &MovableMutex) { - self.0.wait(mutex.raw()) + self.check.verify(mutex); + self.inner.wait(mutex.raw()) } /// Waits for a signal on the specified mutex with a timeout duration /// specified by `dur` (a relative time into the future). /// /// Behavior is undefined if the mutex is not locked by the current thread. - /// Behavior is also undefined if more than one mutex is used concurrently - /// on this condition variable. + /// + /// May panic if used with more than one mutex. #[inline] pub unsafe fn wait_timeout(&self, mutex: &MovableMutex, dur: Duration) -> bool { - self.0.wait_timeout(mutex.raw(), dur) + self.check.verify(mutex); + self.inner.wait_timeout(mutex.raw(), dur) } +} - /// Deallocates all resources associated with this condition variable. - /// - /// Behavior is undefined if there are current or will be future users of - /// this condition variable. - #[inline] - pub unsafe fn destroy(&self) { - self.0.destroy() +impl Drop for Condvar { + fn drop(&mut self) { + unsafe { self.inner.destroy() }; } } diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs new file mode 100644 index 0000000000000..949b53f30b17c --- /dev/null +++ b/library/std/src/sys_common/condvar/check.rs @@ -0,0 +1,23 @@ +use crate::sync::atomic::{AtomicUsize, Ordering}; +use crate::sys::mutex as mutex_imp; +use crate::sys_common::mutex::MovableMutex; + +/// A `Condvar` will check it's only ever used with the same mutex, based on +/// its (stable) address. +pub struct CondvarCheck { + addr: AtomicUsize, +} + +impl CondvarCheck { + pub const fn new() -> Self { + Self { addr: AtomicUsize::new(0) } + } + pub fn verify(&self, mutex: &MovableMutex) { + let addr = mutex.raw() as *const mutex_imp::Mutex as usize; + match self.addr.compare_and_swap(0, addr, Ordering::SeqCst) { + 0 => {} // Stored the address + n if n == addr => {} // Lost a race to store the same address + _ => panic!("attempted to use a condition variable with two mutexes"), + } + } +} diff --git a/library/std/src/sys_common/mutex.rs b/library/std/src/sys_common/mutex.rs index 93ec7d89bc5c7..e30163b3a8949 100644 --- a/library/std/src/sys_common/mutex.rs +++ b/library/std/src/sys_common/mutex.rs @@ -72,7 +72,7 @@ impl MovableMutex { Self(mutex) } - pub(crate) fn raw(&self) -> &imp::Mutex { + pub(super) fn raw(&self) -> &imp::Mutex { &self.0 } From 58deb7001deceb74cec38590a161d781d5d953b4 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:06:35 +0200 Subject: [PATCH 02/12] Make it possible to have unboxed mutexes on specific platforms. This commit keeps all mutexes boxed on all platforms, but makes it trivial to remove the box on some platforms later. --- library/std/src/sys/cloudabi/mutex.rs | 2 ++ library/std/src/sys/sgx/mutex.rs | 2 ++ library/std/src/sys/unix/mutex.rs | 2 ++ library/std/src/sys/unsupported/mutex.rs | 2 ++ library/std/src/sys/vxworks/mutex.rs | 2 ++ library/std/src/sys/wasm/mutex_atomics.rs | 2 ++ library/std/src/sys/windows/mutex.rs | 2 ++ library/std/src/sys_common/condvar.rs | 4 ++- library/std/src/sys_common/condvar/check.rs | 33 ++++++++++++++++++--- library/std/src/sys_common/mutex.rs | 9 +++--- 10 files changed, 51 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/cloudabi/mutex.rs b/library/std/src/sys/cloudabi/mutex.rs index 580ab0e8ad863..651ac3c4cbe7d 100644 --- a/library/std/src/sys/cloudabi/mutex.rs +++ b/library/std/src/sys/cloudabi/mutex.rs @@ -15,6 +15,8 @@ extern "C" { // implemented identically. pub struct Mutex(RWLock); +pub type MovableMutex = Box; + pub unsafe fn raw(m: &Mutex) -> *mut AtomicU32 { rwlock::raw(&m.0) } diff --git a/library/std/src/sys/sgx/mutex.rs b/library/std/src/sys/sgx/mutex.rs index 4911c2f538769..8874517dac60c 100644 --- a/library/std/src/sys/sgx/mutex.rs +++ b/library/std/src/sys/sgx/mutex.rs @@ -8,6 +8,8 @@ pub struct Mutex { inner: SpinMutex>, } +pub type MovableMutex = Box; + // Implementation according to “Operating Systems: Three Easy Pieces”, chapter 28 impl Mutex { pub const fn new() -> Mutex { diff --git a/library/std/src/sys/unix/mutex.rs b/library/std/src/sys/unix/mutex.rs index 45c600f75f5cf..ebc737b75ae0b 100644 --- a/library/std/src/sys/unix/mutex.rs +++ b/library/std/src/sys/unix/mutex.rs @@ -5,6 +5,8 @@ pub struct Mutex { inner: UnsafeCell, } +pub type MovableMutex = Box; + #[inline] pub unsafe fn raw(m: &Mutex) -> *mut libc::pthread_mutex_t { m.inner.get() diff --git a/library/std/src/sys/unsupported/mutex.rs b/library/std/src/sys/unsupported/mutex.rs index 9ef8af52eb5c2..a28f2cf4ffec0 100644 --- a/library/std/src/sys/unsupported/mutex.rs +++ b/library/std/src/sys/unsupported/mutex.rs @@ -4,6 +4,8 @@ pub struct Mutex { locked: UnsafeCell, } +pub type MovableMutex = Box; + unsafe impl Send for Mutex {} unsafe impl Sync for Mutex {} // no threads on this platform diff --git a/library/std/src/sys/vxworks/mutex.rs b/library/std/src/sys/vxworks/mutex.rs index 103d87e3d2f91..dd7582c21a727 100644 --- a/library/std/src/sys/vxworks/mutex.rs +++ b/library/std/src/sys/vxworks/mutex.rs @@ -5,6 +5,8 @@ pub struct Mutex { inner: UnsafeCell, } +pub type MovableMutex = Box; + #[inline] pub unsafe fn raw(m: &Mutex) -> *mut libc::pthread_mutex_t { m.inner.get() diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 4b1a7c9b48141..5d45efe19c290 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -8,6 +8,8 @@ pub struct Mutex { locked: AtomicUsize, } +pub type MovableMutex = Box; + // Mutexes have a pretty simple implementation where they contain an `i32` // internally that is 0 when unlocked and 1 when the mutex is locked. // Acquisition has a fast path where it attempts to cmpxchg the 0 to a 1, and diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index e2aaca59fe2f3..fb6bb9583e29a 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -29,6 +29,8 @@ pub struct Mutex { lock: AtomicUsize, } +pub type MovableMutex = Box; + unsafe impl Send for Mutex {} unsafe impl Sync for Mutex {} diff --git a/library/std/src/sys_common/condvar.rs b/library/std/src/sys_common/condvar.rs index c65f1f81509d2..acd8b69e9ac55 100644 --- a/library/std/src/sys_common/condvar.rs +++ b/library/std/src/sys_common/condvar.rs @@ -1,10 +1,12 @@ use crate::sys::condvar as imp; +use crate::sys::mutex as mutex_imp; use crate::sys_common::mutex::MovableMutex; use crate::time::Duration; -use check::CondvarCheck; mod check; +type CondvarCheck = ::Check; + /// An OS-based condition variable. pub struct Condvar { inner: Box, diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs index 949b53f30b17c..fecb732b910ce 100644 --- a/library/std/src/sys_common/condvar/check.rs +++ b/library/std/src/sys_common/condvar/check.rs @@ -2,13 +2,22 @@ use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sys::mutex as mutex_imp; use crate::sys_common::mutex::MovableMutex; -/// A `Condvar` will check it's only ever used with the same mutex, based on -/// its (stable) address. -pub struct CondvarCheck { +pub trait CondvarCheck { + type Check; +} + +/// For boxed mutexes, a `Condvar` will check it's only ever used with the same +/// mutex, based on its (stable) address. +impl CondvarCheck for Box { + type Check = SameMutexCheck; +} + +pub struct SameMutexCheck { addr: AtomicUsize, } -impl CondvarCheck { +#[allow(dead_code)] +impl SameMutexCheck { pub const fn new() -> Self { Self { addr: AtomicUsize::new(0) } } @@ -21,3 +30,19 @@ impl CondvarCheck { } } } + +/// Unboxed mutexes may move, so `Condvar` can not require its address to stay +/// constant. +impl CondvarCheck for mutex_imp::Mutex { + type Check = NoCheck; +} + +pub struct NoCheck; + +#[allow(dead_code)] +impl NoCheck { + pub const fn new() -> Self { + Self + } + pub fn verify(&self, _: &MovableMutex) {} +} diff --git a/library/std/src/sys_common/mutex.rs b/library/std/src/sys_common/mutex.rs index e30163b3a8949..a1e11d24465ea 100644 --- a/library/std/src/sys_common/mutex.rs +++ b/library/std/src/sys_common/mutex.rs @@ -58,16 +58,17 @@ impl Drop for StaticMutexGuard<'_> { /// /// This mutex does not implement poisoning. /// -/// This is a wrapper around `Box`, to allow the object to be moved -/// without moving the raw mutex. -pub struct MovableMutex(Box); +/// This is either a wrapper around `Box` or `imp::Mutex`, +/// depending on the platform. It is boxed on platforms where `imp::Mutex` may +/// not be moved. +pub struct MovableMutex(imp::MovableMutex); unsafe impl Sync for MovableMutex {} impl MovableMutex { /// Creates a new mutex. pub fn new() -> Self { - let mut mutex = box imp::Mutex::new(); + let mut mutex = imp::MovableMutex::from(imp::Mutex::new()); unsafe { mutex.init() }; Self(mutex) } From def5188ca887e36efdccd2cfda159eb1f3b89f9b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:07:56 +0200 Subject: [PATCH 03/12] No longer put cloudabi mutexes in a box. Cloudabi mutexes may be moved safely. --- library/std/src/sys/cloudabi/mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/cloudabi/mutex.rs b/library/std/src/sys/cloudabi/mutex.rs index 651ac3c4cbe7d..66839e05bf076 100644 --- a/library/std/src/sys/cloudabi/mutex.rs +++ b/library/std/src/sys/cloudabi/mutex.rs @@ -15,7 +15,7 @@ extern "C" { // implemented identically. pub struct Mutex(RWLock); -pub type MovableMutex = Box; +pub type MovableMutex = Mutex; pub unsafe fn raw(m: &Mutex) -> *mut AtomicU32 { rwlock::raw(&m.0) From 2f0386771d17703d8e5ea9abef3359770b51af3f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:08:51 +0200 Subject: [PATCH 04/12] No longer put mutexes on the 'unsupported' platform in a box. These mutexes are just a bool (in a cell), so can be moved without problems. --- library/std/src/sys/unsupported/mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unsupported/mutex.rs b/library/std/src/sys/unsupported/mutex.rs index a28f2cf4ffec0..06ea9a1e2c109 100644 --- a/library/std/src/sys/unsupported/mutex.rs +++ b/library/std/src/sys/unsupported/mutex.rs @@ -4,7 +4,7 @@ pub struct Mutex { locked: UnsafeCell, } -pub type MovableMutex = Box; +pub type MovableMutex = Mutex; unsafe impl Send for Mutex {} unsafe impl Sync for Mutex {} // no threads on this platform From 4f1353e54f9cdca82487b5e69f94e54047d8ea2f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:09:48 +0200 Subject: [PATCH 05/12] No longer put wasm mutexes in a box. These mutexes are just an AtomicUsize, so can be moved without problems. --- library/std/src/sys/wasm/mutex_atomics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 5d45efe19c290..2970fcf806cbf 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -8,7 +8,7 @@ pub struct Mutex { locked: AtomicUsize, } -pub type MovableMutex = Box; +pub type MovableMutex = Mutex; // Mutexes have a pretty simple implementation where they contain an `i32` // internally that is 0 when unlocked and 1 when the mutex is locked. From dc81cbdcb14bfaed773f0cd32f050caa108938e2 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:12:08 +0200 Subject: [PATCH 06/12] No longer put windows mutexes in a box. Windows SRW locks are movable (while not borrowed) according to their documentation. --- library/std/src/sys/windows/mutex.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index fb6bb9583e29a..fa51b006c346f 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -29,7 +29,10 @@ pub struct Mutex { lock: AtomicUsize, } -pub type MovableMutex = Box; +// Windows SRW Locks are movable (while not borrowed). +// ReentrantMutexes (in Inner) are not, but those are stored indirectly through +// a Box, so do not move when the Mutex it self is moved. +pub type MovableMutex = Mutex; unsafe impl Send for Mutex {} unsafe impl Sync for Mutex {} From b181f5a9231c133e2809277932d3a92cb2627b70 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:13:26 +0200 Subject: [PATCH 07/12] Make it possible to have unboxed condvars on specific platforms. This commit keeps all condvars boxed on all platforms, but makes it trivial to remove the box on some platforms later. --- library/std/src/sys/cloudabi/condvar.rs | 2 ++ library/std/src/sys/hermit/condvar.rs | 2 ++ library/std/src/sys/sgx/condvar.rs | 2 ++ library/std/src/sys/unix/condvar.rs | 2 ++ library/std/src/sys/unsupported/condvar.rs | 2 ++ library/std/src/sys/vxworks/condvar.rs | 2 ++ library/std/src/sys/wasm/condvar_atomics.rs | 2 ++ library/std/src/sys/windows/condvar.rs | 2 ++ library/std/src/sys_common/condvar.rs | 4 ++-- 9 files changed, 18 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/cloudabi/condvar.rs b/library/std/src/sys/cloudabi/condvar.rs index dabdc0c9b510a..148a684370e09 100644 --- a/library/std/src/sys/cloudabi/condvar.rs +++ b/library/std/src/sys/cloudabi/condvar.rs @@ -15,6 +15,8 @@ pub struct Condvar { condvar: UnsafeCell, } +pub type MovableCondvar = Box; + unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} diff --git a/library/std/src/sys/hermit/condvar.rs b/library/std/src/sys/hermit/condvar.rs index 52c8c3b17e826..b45e8718f088e 100644 --- a/library/std/src/sys/hermit/condvar.rs +++ b/library/std/src/sys/hermit/condvar.rs @@ -14,6 +14,8 @@ pub struct Condvar { sem2: *const c_void, } +pub type MovableCondvar = Box; + unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} diff --git a/library/std/src/sys/sgx/condvar.rs b/library/std/src/sys/sgx/condvar.rs index ed6dbcf497147..dfe22e6a1b375 100644 --- a/library/std/src/sys/sgx/condvar.rs +++ b/library/std/src/sys/sgx/condvar.rs @@ -7,6 +7,8 @@ pub struct Condvar { inner: SpinMutex>, } +pub type MovableCondvar = Box; + impl Condvar { pub const fn new() -> Condvar { Condvar { inner: SpinMutex::new(WaitVariable::new(())) } diff --git a/library/std/src/sys/unix/condvar.rs b/library/std/src/sys/unix/condvar.rs index 9f1847943f326..e38f91af9f0b9 100644 --- a/library/std/src/sys/unix/condvar.rs +++ b/library/std/src/sys/unix/condvar.rs @@ -6,6 +6,8 @@ pub struct Condvar { inner: UnsafeCell, } +pub type MovableCondvar = Box; + unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} diff --git a/library/std/src/sys/unsupported/condvar.rs b/library/std/src/sys/unsupported/condvar.rs index a578eee8ccce2..e49f21bef785a 100644 --- a/library/std/src/sys/unsupported/condvar.rs +++ b/library/std/src/sys/unsupported/condvar.rs @@ -3,6 +3,8 @@ use crate::time::Duration; pub struct Condvar {} +pub type MovableCondvar = Box; + impl Condvar { pub const fn new() -> Condvar { Condvar {} diff --git a/library/std/src/sys/vxworks/condvar.rs b/library/std/src/sys/vxworks/condvar.rs index 5a77966d97468..b4724be7c7c3b 100644 --- a/library/std/src/sys/vxworks/condvar.rs +++ b/library/std/src/sys/vxworks/condvar.rs @@ -6,6 +6,8 @@ pub struct Condvar { inner: UnsafeCell, } +pub type MovableCondvar = Box; + unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index d86bb60507be2..304c3bb4c58aa 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -9,6 +9,8 @@ pub struct Condvar { cnt: AtomicUsize, } +pub type MovableCondvar = Box; + // Condition variables are implemented with a simple counter internally that is // likely to cause spurious wakeups. Blocking on a condition variable will first // read the value of the internal counter, unlock the given mutex, and then diff --git a/library/std/src/sys/windows/condvar.rs b/library/std/src/sys/windows/condvar.rs index 8f7f6854cc22c..2b2ff13c78b81 100644 --- a/library/std/src/sys/windows/condvar.rs +++ b/library/std/src/sys/windows/condvar.rs @@ -8,6 +8,8 @@ pub struct Condvar { inner: UnsafeCell, } +pub type MovableCondvar = Box; + unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} diff --git a/library/std/src/sys_common/condvar.rs b/library/std/src/sys_common/condvar.rs index acd8b69e9ac55..2c02e1cd33c81 100644 --- a/library/std/src/sys_common/condvar.rs +++ b/library/std/src/sys_common/condvar.rs @@ -9,14 +9,14 @@ type CondvarCheck = ::Check; /// An OS-based condition variable. pub struct Condvar { - inner: Box, + inner: imp::MovableCondvar, check: CondvarCheck, } impl Condvar { /// Creates a new condition variable for use. pub fn new() -> Self { - let mut c = box imp::Condvar::new(); + let mut c = imp::MovableCondvar::from(imp::Condvar::new()); unsafe { c.init() }; Self { inner: c, check: CondvarCheck::new() } } From 5769a46788ec00338519fd8f87558419f0d6dd67 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:07:56 +0200 Subject: [PATCH 08/12] No longer put cloudabi condvars in a box. Cloudabi condvars may be moved safely. --- library/std/src/sys/cloudabi/condvar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/cloudabi/condvar.rs b/library/std/src/sys/cloudabi/condvar.rs index 148a684370e09..82d89b260fafd 100644 --- a/library/std/src/sys/cloudabi/condvar.rs +++ b/library/std/src/sys/cloudabi/condvar.rs @@ -15,7 +15,7 @@ pub struct Condvar { condvar: UnsafeCell, } -pub type MovableCondvar = Box; +pub type MovableCondvar = Condvar; unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} From 7f56a354114dffbbee8b12f134518d50d3aefe6a Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:08:51 +0200 Subject: [PATCH 09/12] No longer put condvars on the 'unsupported' platform in a box. These condvars are unsupported and implemented as a ZST, so can be moved without problems. --- library/std/src/sys/unsupported/condvar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unsupported/condvar.rs b/library/std/src/sys/unsupported/condvar.rs index e49f21bef785a..35d12a69c8aa5 100644 --- a/library/std/src/sys/unsupported/condvar.rs +++ b/library/std/src/sys/unsupported/condvar.rs @@ -3,7 +3,7 @@ use crate::time::Duration; pub struct Condvar {} -pub type MovableCondvar = Box; +pub type MovableCondvar = Condvar; impl Condvar { pub const fn new() -> Condvar { From ec69a858e44992b37af3d559479777c0dfec1c7a Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:09:48 +0200 Subject: [PATCH 10/12] No longer put wasm condvars in a box. These condvars are just an AtomicUsize, so can be moved without problems. --- library/std/src/sys/wasm/condvar_atomics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index 304c3bb4c58aa..a96bb18e6ef1a 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -9,7 +9,7 @@ pub struct Condvar { cnt: AtomicUsize, } -pub type MovableCondvar = Box; +pub type MovableCondvar = Condvar; // Condition variables are implemented with a simple counter internally that is // likely to cause spurious wakeups. Blocking on a condition variable will first From f3837e788b8448e9655797bef73ab31dcf985ac1 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:12:08 +0200 Subject: [PATCH 11/12] No longer put windows condvars in a box. Windows condition variables are movable (while not borrowed) according to their documentation. --- library/std/src/sys/windows/condvar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/windows/condvar.rs b/library/std/src/sys/windows/condvar.rs index 2b2ff13c78b81..44547a5c51a34 100644 --- a/library/std/src/sys/windows/condvar.rs +++ b/library/std/src/sys/windows/condvar.rs @@ -8,7 +8,7 @@ pub struct Condvar { inner: UnsafeCell, } -pub type MovableCondvar = Box; +pub type MovableCondvar = Condvar; unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} From b1ce7a38a6c03ddff23ef7e59e74cab6452ed9b0 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 1 Oct 2020 01:51:32 +0200 Subject: [PATCH 12/12] Disable condvar::two_mutexes test on non-unix platforms. Condvars are no longer guaranteed to panic in this case on all platforms. At least the unix implementation still does. --- library/std/src/sync/condvar/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sync/condvar/tests.rs b/library/std/src/sync/condvar/tests.rs index 86d099ee3a19c..6757707cd9513 100644 --- a/library/std/src/sync/condvar/tests.rs +++ b/library/std/src/sync/condvar/tests.rs @@ -191,7 +191,7 @@ fn wait_timeout_wake() { #[test] #[should_panic] -#[cfg_attr(target_os = "emscripten", ignore)] +#[cfg_attr(not(unix), ignore)] fn two_mutexes() { let m = Arc::new(Mutex::new(())); let m2 = m.clone();