Rollup merge of rust-lang#84687 - a1phyr:improve_rwlock, r=m-ou-se

Multiple improvements to RwLocks

This PR replicates rust-lang#77147, rust-lang#77380 and rust-lang#84650 on RWLocks :
- Split `sys_common::RWLock` in `StaticRWLock` and `MovableRWLock`
- Unbox rwlocks on some platforms (Windows, Wasm and unsupported)
- Simplify `RwLock::into_inner`

Notes to reviewers :
- For each target, I copied `MovableMutex` to guess if `MovableRWLock` should be boxed.
- ~A comment says that `StaticMutex` is not re-entrant, I don't understand why and I don't know whether it applies to `StaticRWLock`.~

r? `@m-ou-se`

library/std/src/panicking.rs

@@ -19,7 +19,7 @@ use crate::process;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::sys::stdio::panic_output;
 use crate::sys_common::backtrace::{self, RustBacktrace};
-use crate::sys_common::rwlock::RWLock;
+use crate::sys_common::rwlock::StaticRWLock;
 use crate::sys_common::thread_info;
 use crate::thread;
 
@@ -74,7 +74,7 @@ enum Hook {
     Custom(*mut (dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send)),
 }
 
-static HOOK_LOCK: RWLock = RWLock::new();
+static HOOK_LOCK: StaticRWLock = StaticRWLock::new();
 static mut HOOK: Hook = Hook::Default;
 
 /// Registers a custom panic hook, replacing any that was previously registered.
@@ -117,10 +117,10 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) {
     }
 
     unsafe {
-        HOOK_LOCK.write();
+        let guard = HOOK_LOCK.write();
         let old_hook = HOOK;
         HOOK = Hook::Custom(Box::into_raw(hook));
-        HOOK_LOCK.write_unlock();
+        drop(guard);
 
         if let Hook::Custom(ptr) = old_hook {
             #[allow(unused_must_use)]
@@ -165,10 +165,10 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> {
     }
 
     unsafe {
-        HOOK_LOCK.write();
+        let guard = HOOK_LOCK.write();
         let hook = HOOK;
         HOOK = Hook::Default;
-        HOOK_LOCK.write_unlock();
+        drop(guard);
 
         match hook {
             Hook::Default => Box::new(default_hook),
@@ -608,7 +608,7 @@ fn rust_panic_with_hook(
 
     unsafe {
         let mut info = PanicInfo::internal_constructor(message, location);
-        HOOK_LOCK.read();
+        let _guard = 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
@@ -626,7 +626,6 @@ fn rust_panic_with_hook(
                 (*ptr)(&info);
             }
         };
-        HOOK_LOCK.read_unlock();
     }
 
     if panics > 1 {

library/std/src/sync/rwlock.rs

@@ -3,9 +3,7 @@ mod tests;
 
 use crate::cell::UnsafeCell;
 use crate::fmt;
-use crate::mem;
 use crate::ops::{Deref, DerefMut};
-use crate::ptr;
 use crate::sync::{poison, LockResult, TryLockError, TryLockResult};
 use crate::sys_common::rwlock as sys;
 
@@ -66,7 +64,7 @@ use crate::sys_common::rwlock as sys;
 /// [`Mutex`]: super::Mutex
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct RwLock<T: ?Sized> {
-    inner: Box<sys::RWLock>,
+    inner: sys::MovableRWLock,
     poison: poison::Flag,
     data: UnsafeCell<T>,
 }
@@ -130,7 +128,7 @@ impl<T> RwLock<T> {
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn new(t: T) -> RwLock<T> {
         RwLock {
-            inner: box sys::RWLock::new(),
+            inner: sys::MovableRWLock::new(),
             poison: poison::Flag::new(),
             data: UnsafeCell::new(t),
         }
@@ -376,24 +374,8 @@ impl<T: ?Sized> RwLock<T> {
     where
         T: Sized,
     {
-        // We know statically that there are no outstanding references to
-        // `self` so there's no need to lock the inner lock.
-        //
-        // To get the inner value, we'd like to call `data.into_inner()`,
-        // but because `RwLock` impl-s `Drop`, we can't move out of it, so
-        // we'll have to destructure it manually instead.
-        unsafe {
-            // Like `let RwLock { inner, poison, data } = self`.
-            let (inner, poison, data) = {
-                let RwLock { ref inner, ref poison, ref data } = self;
-                (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())
-        }
+        let data = self.data.into_inner();
+        poison::map_result(self.poison.borrow(), |_| data)
     }
 
     /// Returns a mutable reference to the underlying data.
@@ -424,14 +406,6 @@ impl<T: ?Sized> RwLock<T> {
     }
 }
 
-#[stable(feature = "rust1", since = "1.0.0")]
-unsafe impl<#[may_dangle] T: ?Sized> Drop for RwLock<T> {
-    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<T: ?Sized + fmt::Debug> fmt::Debug for RwLock<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

library/std/src/sys/hermit/rwlock.rs

@@ -8,6 +8,8 @@ pub struct RWLock {
     state: UnsafeCell<State>,
 }
 
+pub type MovableRWLock = Box<RWLock>;
+
 enum State {
     Unlocked,
     Reading(usize),

library/std/src/sys/sgx/rwlock.rs

@@ -13,6 +13,8 @@ pub struct RWLock {
     writer: SpinMutex<WaitVariable<bool>>,
 }
 
+pub type MovableRWLock = Box<RWLock>;
+
 // Check at compile time that RWLock size matches C definition (see test_c_rwlock_initializer below)
 //
 // # Safety

library/std/src/sys/unix/os.rs

@@ -20,8 +20,7 @@ use crate::str;
 use crate::sys::cvt;
 use crate::sys::fd;
 use crate::sys::memchr;
-use crate::sys::rwlock::{RWLockReadGuard, StaticRWLock};
-use crate::sys_common::mutex::{StaticMutex, StaticMutexGuard};
+use crate::sys_common::rwlock::{StaticRWLock, StaticRWLockReadGuard};
 use crate::vec;
 
 use libc::{c_char, c_int, c_void};
@@ -490,8 +489,8 @@ pub unsafe fn environ() -> *mut *const *const c_char {
 
 static ENV_LOCK: StaticRWLock = StaticRWLock::new();
 
-pub fn env_read_lock() -> RWLockReadGuard {
-    ENV_LOCK.read_with_guard()
+pub fn env_read_lock() -> StaticRWLockReadGuard {
+    ENV_LOCK.read()
 }
 
 /// Returns a vector of (variable, value) byte-vector pairs for all the
@@ -551,7 +550,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let v = CString::new(v.as_bytes())?;
 
     unsafe {
-        let _guard = ENV_LOCK.write_with_guard();
+        let _guard = ENV_LOCK.write();
         cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
     }
 }
@@ -560,7 +559,7 @@ pub fn unsetenv(n: &OsStr) -> io::Result<()> {
     let nbuf = CString::new(n.as_bytes())?;
 
     unsafe {
-        let _guard = ENV_LOCK.write_with_guard();
+        let _guard = ENV_LOCK.write();
         cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
     }
 }

library/std/src/sys/unix/rwlock.rs

@@ -7,6 +7,8 @@ pub struct RWLock {
     num_readers: AtomicUsize,
 }
 
+pub type MovableRWLock = Box<RWLock>;
+
 unsafe impl Send for RWLock {}
 unsafe impl Sync for RWLock {}
 
@@ -139,55 +141,3 @@ impl RWLock {
         }
     }
 }
-
-pub struct StaticRWLock(RWLock);
-
-impl StaticRWLock {
-    pub const fn new() -> StaticRWLock {
-        StaticRWLock(RWLock::new())
-    }
-
-    /// Acquires shared access to the underlying lock, blocking the current
-    /// thread to do so.
-    ///
-    /// The lock is automatically unlocked when the returned guard is dropped.
-    #[inline]
-    pub fn read_with_guard(&'static self) -> RWLockReadGuard {
-        // SAFETY: All methods require static references, therefore self
-        // cannot be moved between invocations.
-        unsafe {
-            self.0.read();
-        }
-        RWLockReadGuard(&self.0)
-    }
-
-    /// Acquires write access to the underlying lock, blocking the current thread
-    /// to do so.
-    ///
-    /// The lock is automatically unlocked when the returned guard is dropped.
-    #[inline]
-    pub fn write_with_guard(&'static self) -> RWLockWriteGuard {
-        // SAFETY: All methods require static references, therefore self
-        // cannot be moved between invocations.
-        unsafe {
-            self.0.write();
-        }
-        RWLockWriteGuard(&self.0)
-    }
-}
-
-pub struct RWLockReadGuard(&'static RWLock);
-
-impl Drop for RWLockReadGuard {
-    fn drop(&mut self) {
-        unsafe { self.0.read_unlock() }
-    }
-}
-
-pub struct RWLockWriteGuard(&'static RWLock);
-
-impl Drop for RWLockWriteGuard {
-    fn drop(&mut self) {
-        unsafe { self.0.write_unlock() }
-    }
-}

library/std/src/sys/unsupported/rwlock.rs

@@ -5,6 +5,8 @@ pub struct RWLock {
     mode: Cell<isize>,
 }
 
+pub type MovableRWLock = RWLock;
+
 unsafe impl Send for RWLock {}
 unsafe impl Sync for RWLock {} // no threads on this platform
 

library/std/src/sys/wasm/atomics/rwlock.rs

@@ -8,6 +8,8 @@ pub struct RWLock {
     state: UnsafeCell<State>,
 }
 
+pub type MovableRWLock = RWLock;
+
 enum State {
     Unlocked,
     Reading(usize),

library/std/src/sys/windows/rwlock.rs

@@ -5,6 +5,8 @@ pub struct RWLock {
     inner: UnsafeCell<c::SRWLOCK>,
 }
 
+pub type MovableRWLock = RWLock;
+
 unsafe impl Send for RWLock {}
 unsafe impl Sync for RWLock {}