From 58756573fc5a2198f98e9b4889c494bb60f38e3d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 7 Oct 2020 14:06:17 +0200 Subject: [PATCH 1/2] Fix comment about non-reentrant StaticMutex::lock(). The comment said it's UB to call lock() while it is locked. That'd be quite a useless Mutex. :) It was supposed to say 'locked by the same thread', not just 'locked'. --- library/std/src/sys_common/mutex.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys_common/mutex.rs b/library/std/src/sys_common/mutex.rs index 91d919a3f9b..47fa7539330 100644 --- a/library/std/src/sys_common/mutex.rs +++ b/library/std/src/sys_common/mutex.rs @@ -28,8 +28,9 @@ impl StaticMutex { /// Calls raw_lock() and then returns an RAII guard to guarantee the mutex /// will be unlocked. /// - /// It is undefined behaviour to call this function while locked, or if the - /// mutex has been moved since the last time this was called. + /// It is undefined behaviour to call this function while locked by the + /// same thread, or if the mutex has been moved since the last time this + /// was called. #[inline] pub unsafe fn lock(&self) -> StaticMutexGuard<'_> { self.0.lock(); From 44a2af32ccc4c71c5e8bc9cb79c9042c6e608ce8 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 7 Oct 2020 14:08:33 +0200 Subject: [PATCH 2/2] Remove lifetime from StaticMutex and assume 'static. StaticMutex is only ever used with as a static (as the name already suggests). So it doesn't have to be generic over a lifetime, but can simply assume 'static. This 'static lifetime guarantees the object is never moved, so this is no longer a manually checked requirement for unsafe calls to lock(). --- library/std/src/sys/unix/os.rs | 2 +- library/std/src/sys/vxworks/os.rs | 2 +- library/std/src/sys_common/mutex.rs | 17 +++++------------ 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index c9f9ed01e12..2392238d0a1 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -470,7 +470,7 @@ pub unsafe fn environ() -> *mut *const *const c_char { &mut environ } -pub unsafe fn env_lock() -> StaticMutexGuard<'static> { +pub unsafe fn env_lock() -> StaticMutexGuard { // It is UB to attempt to acquire this mutex reentrantly! static ENV_LOCK: StaticMutex = StaticMutex::new(); ENV_LOCK.lock() diff --git a/library/std/src/sys/vxworks/os.rs b/library/std/src/sys/vxworks/os.rs index 08394a8d29d..6eaec6f1e50 100644 --- a/library/std/src/sys/vxworks/os.rs +++ b/library/std/src/sys/vxworks/os.rs @@ -212,7 +212,7 @@ pub unsafe fn environ() -> *mut *const *const c_char { &mut environ } -pub unsafe fn env_lock() -> StaticMutexGuard<'static> { +pub unsafe fn env_lock() -> StaticMutexGuard { // It is UB to attempt to acquire this mutex reentrantly! static ENV_LOCK: StaticMutex = StaticMutex::new(); ENV_LOCK.lock() diff --git a/library/std/src/sys_common/mutex.rs b/library/std/src/sys_common/mutex.rs index 47fa7539330..f3e7efb955a 100644 --- a/library/std/src/sys_common/mutex.rs +++ b/library/std/src/sys_common/mutex.rs @@ -3,8 +3,7 @@ use crate::sys::mutex as imp; /// An OS-based mutual exclusion lock, meant for use in static variables. /// /// This mutex has a const constructor ([`StaticMutex::new`]), does not -/// implement `Drop` to cleanup resources, and causes UB when moved or used -/// reentrantly. +/// implement `Drop` to cleanup resources, and causes UB when used reentrantly. /// /// This mutex does not implement poisoning. /// @@ -16,11 +15,6 @@ unsafe impl Sync for StaticMutex {} impl StaticMutex { /// Creates a new mutex for use. - /// - /// Behavior is undefined if the mutex is moved after it is - /// first used with any of the functions below. - /// Also, the behavior is undefined if this mutex is ever used reentrantly, - /// i.e., `lock` is called by the thread currently holding the lock. pub const fn new() -> Self { Self(imp::Mutex::new()) } @@ -29,19 +23,18 @@ impl StaticMutex { /// will be unlocked. /// /// It is undefined behaviour to call this function while locked by the - /// same thread, or if the mutex has been moved since the last time this - /// was called. + /// same thread. #[inline] - pub unsafe fn lock(&self) -> StaticMutexGuard<'_> { + pub unsafe fn lock(&'static self) -> StaticMutexGuard { self.0.lock(); StaticMutexGuard(&self.0) } } #[must_use] -pub struct StaticMutexGuard<'a>(&'a imp::Mutex); +pub struct StaticMutexGuard(&'static imp::Mutex); -impl Drop for StaticMutexGuard<'_> { +impl Drop for StaticMutexGuard { #[inline] fn drop(&mut self) { unsafe {