From 9b2112ab70f366d57b3d140a35257745c6535767 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 12 Oct 2020 20:15:27 +0200 Subject: [PATCH 1/2] Use Pin to pin RwLock. --- library/std/src/lib.rs | 1 + library/std/src/panicking.rs | 13 ++++--- library/std/src/sync/rwlock.rs | 58 ++++++++++++---------------- library/std/src/sys_common/rwlock.rs | 45 ++++++++------------- 4 files changed, 50 insertions(+), 67 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 30e7a7f3c3b10..3460f945328f1 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -289,6 +289,7 @@ #![feature(panic_info_message)] #![feature(panic_internals)] #![feature(panic_unwind)] +#![feature(pin_static_ref)] #![feature(prelude_import)] #![feature(ptr_internals)] #![feature(raw)] diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 8dceb12de87b8..6e7cbef2acf73 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -15,6 +15,7 @@ use crate::any::Any; use crate::fmt; use crate::intrinsics; use crate::mem::{self, ManuallyDrop}; +use crate::pin::Pin; use crate::process; use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys::stdio::panic_output; @@ -116,11 +117,11 @@ pub fn set_hook(hook: Box) + 'static + Sync + Send>) { panic!("cannot modify the panic hook from a panicking thread"); } + Pin::static_ref(&HOOK_LOCK).write(); unsafe { - HOOK_LOCK.write(); let old_hook = HOOK; HOOK = Hook::Custom(Box::into_raw(hook)); - HOOK_LOCK.write_unlock(); + Pin::static_ref(&HOOK_LOCK).write_unlock(); if let Hook::Custom(ptr) = old_hook { #[allow(unused_must_use)] @@ -164,11 +165,11 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { panic!("cannot modify the panic hook from a panicking thread"); } + Pin::static_ref(&HOOK_LOCK).write(); unsafe { - HOOK_LOCK.write(); let hook = HOOK; HOOK = Hook::Default; - HOOK_LOCK.write_unlock(); + Pin::static_ref(&HOOK_LOCK).write_unlock(); match hook { Hook::Default => Box::new(default_hook), @@ -563,7 +564,7 @@ fn rust_panic_with_hook( unsafe { let mut info = PanicInfo::internal_constructor(message, location); - HOOK_LOCK.read(); + Pin::static_ref(&HOOK_LOCK).read(); match HOOK { // Some platforms (like wasm) know that printing to stderr won't ever actually // print anything, and if that's the case we can skip the default @@ -581,7 +582,7 @@ fn rust_panic_with_hook( (*ptr)(&info); } }; - HOOK_LOCK.read_unlock(); + Pin::static_ref(&HOOK_LOCK).read_unlock(); } if panics > 1 { diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index d967779ce361d..7455b28b907c0 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -5,6 +5,7 @@ use crate::cell::UnsafeCell; use crate::fmt; use crate::mem; use crate::ops::{Deref, DerefMut}; +use crate::pin::Pin; use crate::ptr; use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; use crate::sys_common::rwlock as sys; @@ -64,7 +65,7 @@ use crate::sys_common::rwlock as sys; /// [`Mutex`]: super::Mutex #[stable(feature = "rust1", since = "1.0.0")] pub struct RwLock { - inner: Box, + inner: Pin>, poison: poison::Flag, data: UnsafeCell, } @@ -128,7 +129,7 @@ impl RwLock { #[stable(feature = "rust1", since = "1.0.0")] pub fn new(t: T) -> RwLock { RwLock { - inner: box sys::RWLock::new(), + inner: Box::pin(sys::RWLock::new()), poison: poison::Flag::new(), data: UnsafeCell::new(t), } @@ -178,10 +179,9 @@ impl RwLock { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn read(&self) -> LockResult> { - unsafe { - self.inner.read(); - RwLockReadGuard::new(self) - } + self.inner.as_ref().read(); + // SAFETY: We've gotten a read-lock on the rwlock. + unsafe { RwLockReadGuard::new(self) } } /// Attempts to acquire this rwlock with shared read access. @@ -217,12 +217,11 @@ impl RwLock { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn try_read(&self) -> TryLockResult> { - unsafe { - if self.inner.try_read() { - Ok(RwLockReadGuard::new(self)?) - } else { - Err(TryLockError::WouldBlock) - } + if self.inner.as_ref().try_read() { + // SAFETY: We've gotten a read-lock on the rwlock. + unsafe { Ok(RwLockReadGuard::new(self)?) } + } else { + Err(TryLockError::WouldBlock) } } @@ -260,10 +259,9 @@ impl RwLock { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn write(&self) -> LockResult> { - unsafe { - self.inner.write(); - RwLockWriteGuard::new(self) - } + self.inner.as_ref().write(); + // SAFETY: We've gotten a write-lock on the rwlock. + unsafe { RwLockWriteGuard::new(self) } } /// Attempts to lock this rwlock with exclusive write access. @@ -299,12 +297,11 @@ impl RwLock { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn try_write(&self) -> TryLockResult> { - unsafe { - if self.inner.try_write() { - Ok(RwLockWriteGuard::new(self)?) - } else { - Err(TryLockError::WouldBlock) - } + if self.inner.as_ref().try_write() { + // SAFETY: We've gotten a write-lock on the rwlock. + unsafe { Ok(RwLockWriteGuard::new(self)?) } + } else { + Err(TryLockError::WouldBlock) } } @@ -374,7 +371,6 @@ impl RwLock { (ptr::read(inner), ptr::read(poison), ptr::read(data)) }; mem::forget(self); - inner.destroy(); // Keep in sync with the `Drop` impl. drop(inner); poison::map_result(poison.borrow(), |_| data.into_inner()) @@ -409,14 +405,6 @@ impl RwLock { } } -#[stable(feature = "rust1", since = "1.0.0")] -unsafe impl<#[may_dangle] T: ?Sized> Drop for RwLock { - fn drop(&mut self) { - // IMPORTANT: This code needs to be kept in sync with `RwLock::into_inner`. - unsafe { self.inner.destroy() } - } -} - #[stable(feature = "rust1", since = "1.0.0")] impl fmt::Debug for RwLock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -524,8 +512,10 @@ impl DerefMut for RwLockWriteGuard<'_, T> { #[stable(feature = "rust1", since = "1.0.0")] impl Drop for RwLockReadGuard<'_, T> { fn drop(&mut self) { + // SAFETY: The fact that we have a RwLockReadGuard proves this thread + // has this rwlock read-locked. unsafe { - self.lock.inner.read_unlock(); + self.lock.inner.as_ref().read_unlock(); } } } @@ -534,8 +524,10 @@ impl Drop for RwLockReadGuard<'_, T> { impl Drop for RwLockWriteGuard<'_, T> { fn drop(&mut self) { self.lock.poison.done(&self.poison); + // SAFETY: The fact that we have a RwLockWriteGuard proves this thread + // has this rwlock write-locked. unsafe { - self.lock.inner.write_unlock(); + self.lock.inner.as_ref().write_unlock(); } } } diff --git a/library/std/src/sys_common/rwlock.rs b/library/std/src/sys_common/rwlock.rs index 3705d641a1be6..9f88acb3bef3d 100644 --- a/library/std/src/sys_common/rwlock.rs +++ b/library/std/src/sys_common/rwlock.rs @@ -1,3 +1,4 @@ +use crate::pin::Pin; use crate::sys::rwlock as imp; /// An OS-based reader-writer lock. @@ -18,53 +19,41 @@ impl RWLock { /// Acquires shared access to the underlying lock, blocking the current /// thread to do so. - /// - /// Behavior is undefined if the rwlock has been moved between this and any - /// previous method call. #[inline] - pub unsafe fn read(&self) { - self.0.read() + pub fn read(self: Pin<&Self>) { + unsafe { self.0.read() } } /// Attempts to acquire shared access to this lock, returning whether it /// succeeded or not. /// /// This function does not block the current thread. - /// - /// Behavior is undefined if the rwlock has been moved between this and any - /// previous method call. #[inline] - pub unsafe fn try_read(&self) -> bool { - self.0.try_read() + pub fn try_read(self: Pin<&Self>) -> bool { + unsafe { self.0.try_read() } } /// Acquires write access to the underlying lock, blocking the current thread /// to do so. - /// - /// Behavior is undefined if the rwlock has been moved between this and any - /// previous method call. #[inline] - pub unsafe fn write(&self) { - self.0.write() + pub fn write(self: Pin<&Self>) { + unsafe { self.0.write() } } /// Attempts to acquire exclusive access to this lock, returning whether it /// succeeded or not. /// /// This function does not block the current thread. - /// - /// Behavior is undefined if the rwlock has been moved between this and any - /// previous method call. #[inline] - pub unsafe fn try_write(&self) -> bool { - self.0.try_write() + pub fn try_write(self: Pin<&Self>) -> bool { + unsafe { self.0.try_write() } } /// Unlocks previously acquired shared access to this lock. /// /// Behavior is undefined if the current thread does not have shared access. #[inline] - pub unsafe fn read_unlock(&self) { + pub unsafe fn read_unlock(self: Pin<&Self>) { self.0.read_unlock() } @@ -73,16 +62,16 @@ impl RWLock { /// Behavior is undefined if the current thread does not currently have /// exclusive access. #[inline] - pub unsafe fn write_unlock(&self) { + pub unsafe fn write_unlock(self: Pin<&Self>) { self.0.write_unlock() } +} - /// Destroys OS-related resources with this RWLock. - /// - /// Behavior is undefined if there are any currently active users of this - /// lock. +impl Drop for RWLock { #[inline] - pub unsafe fn destroy(&self) { - self.0.destroy() + fn drop(&mut self) { + // SAFETY: The rwlock wasn't moved since using any of its + // functions, because they all require a Pin. + unsafe { self.0.destroy() } } } From 9b48a5faaf194e28d8ecc8bfd5427309be5303c2 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 12 Oct 2020 20:50:16 +0200 Subject: [PATCH 2/2] Use PhantomPinned in RWLock to enforce Pin usage. --- library/std/src/sys_common/rwlock.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys_common/rwlock.rs b/library/std/src/sys_common/rwlock.rs index 9f88acb3bef3d..2f913a5d33fa2 100644 --- a/library/std/src/sys_common/rwlock.rs +++ b/library/std/src/sys_common/rwlock.rs @@ -1,3 +1,4 @@ +use crate::marker::PhantomPinned; use crate::pin::Pin; use crate::sys::rwlock as imp; @@ -6,7 +7,10 @@ use crate::sys::rwlock as imp; /// This structure is entirely unsafe and serves as the lowest layer of a /// cross-platform binding of system rwlocks. It is recommended to use the /// safer types at the top level of this crate instead of this type. -pub struct RWLock(imp::RWLock); +pub struct RWLock { + inner: imp::RWLock, + _pinned: PhantomPinned, +} impl RWLock { /// Creates a new reader-writer lock for use. @@ -14,14 +18,14 @@ impl RWLock { /// Behavior is undefined if the reader-writer lock is moved after it is /// first used with any of the functions below. pub const fn new() -> RWLock { - RWLock(imp::RWLock::new()) + RWLock { inner: imp::RWLock::new(), _pinned: PhantomPinned } } /// Acquires shared access to the underlying lock, blocking the current /// thread to do so. #[inline] pub fn read(self: Pin<&Self>) { - unsafe { self.0.read() } + unsafe { self.inner.read() } } /// Attempts to acquire shared access to this lock, returning whether it @@ -30,14 +34,14 @@ impl RWLock { /// This function does not block the current thread. #[inline] pub fn try_read(self: Pin<&Self>) -> bool { - unsafe { self.0.try_read() } + unsafe { self.inner.try_read() } } /// Acquires write access to the underlying lock, blocking the current thread /// to do so. #[inline] pub fn write(self: Pin<&Self>) { - unsafe { self.0.write() } + unsafe { self.inner.write() } } /// Attempts to acquire exclusive access to this lock, returning whether it @@ -46,7 +50,7 @@ impl RWLock { /// This function does not block the current thread. #[inline] pub fn try_write(self: Pin<&Self>) -> bool { - unsafe { self.0.try_write() } + unsafe { self.inner.try_write() } } /// Unlocks previously acquired shared access to this lock. @@ -54,7 +58,7 @@ impl RWLock { /// Behavior is undefined if the current thread does not have shared access. #[inline] pub unsafe fn read_unlock(self: Pin<&Self>) { - self.0.read_unlock() + self.inner.read_unlock() } /// Unlocks previously acquired exclusive access to this lock. @@ -63,7 +67,7 @@ impl RWLock { /// exclusive access. #[inline] pub unsafe fn write_unlock(self: Pin<&Self>) { - self.0.write_unlock() + self.inner.write_unlock() } } @@ -72,6 +76,6 @@ impl Drop for RWLock { fn drop(&mut self) { // SAFETY: The rwlock wasn't moved since using any of its // functions, because they all require a Pin. - unsafe { self.0.destroy() } + unsafe { self.inner.destroy() } } }