std: do not wrap platform locks in sys_common; remove StaticMutex and StaticRwLock

Author: joboet

library/std/src/backtrace.rs

@@ -325,8 +325,7 @@ impl Backtrace {
     // Capture a backtrace which start just before the function addressed by
     // `ip`
     fn create(ip: usize) -> Backtrace {
-        // SAFETY: We don't attempt to lock this reentrantly.
-        let _lock = unsafe { lock() };
+        let _lock = lock();
         let mut frames = Vec::new();
         let mut actual_start = None;
         unsafe {
@@ -469,8 +468,7 @@ impl Capture {
         // Use the global backtrace lock to synchronize this as it's a
         // requirement of the `backtrace` crate, and then actually resolve
         // everything.
-        // SAFETY: We don't attempt to lock this reentrantly.
-        let _lock = unsafe { lock() };
+        let _lock = lock();
         for frame in self.frames.iter_mut() {
             let symbols = &mut frame.symbols;
             let frame = match &frame.frame {

library/std/src/io/stdio.rs

@@ -8,7 +8,6 @@ use crate::io::prelude::*;
 use crate::cell::{Cell, RefCell};
 use crate::fmt;
 use crate::io::{self, BufReader, IoSlice, IoSliceMut, LineWriter, Lines};
-use crate::pin::Pin;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::sync::{Arc, Mutex, MutexGuard, OnceLock};
 use crate::sys::stdio;
@@ -526,7 +525,7 @@ pub struct Stdout {
     // FIXME: this should be LineWriter or BufWriter depending on the state of
     //        stdout (tty or not). Note that if this is not line buffered it
     //        should also flush-on-panic or some form of flush-on-abort.
-    inner: Pin<&'static ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>>,
+    inner: &'static ReentrantMutex<RefCell<Option<LineWriter<StdoutRaw>>>>,
 }
 
 /// A locked reference to the [`Stdout`] handle.
@@ -548,10 +547,11 @@ pub struct Stdout {
 #[must_use = "if unused stdout will immediately unlock"]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct StdoutLock<'a> {
-    inner: ReentrantMutexGuard<'a, RefCell<LineWriter<StdoutRaw>>>,
+    inner: ReentrantMutexGuard<'a, RefCell<Option<LineWriter<StdoutRaw>>>>,
 }
 
-static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLock::new();
+static STDOUT: ReentrantMutex<RefCell<Option<LineWriter<StdoutRaw>>>> =
+    ReentrantMutex::new(RefCell::new(None));
 
 /// Constructs a new handle to the standard output of the current process.
 ///
@@ -602,25 +602,18 @@ static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLo
 #[must_use]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn stdout() -> Stdout {
-    Stdout {
-        inner: Pin::static_ref(&STDOUT).get_or_init_pin(
-            || unsafe { ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))) },
-            |mutex| unsafe { mutex.init() },
-        ),
-    }
+    Stdout { inner: &STDOUT }
 }
 
 pub fn cleanup() {
-    if let Some(instance) = STDOUT.get() {
-        // Flush the data and disable buffering during shutdown
-        // by replacing the line writer by one with zero
-        // buffering capacity.
-        // We use try_lock() instead of lock(), because someone
-        // might have leaked a StdoutLock, which would
-        // otherwise cause a deadlock here.
-        if let Some(lock) = Pin::static_ref(instance).try_lock() {
-            *lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
-        }
+    // Flush the data and disable buffering during shutdown
+    // by replacing the line writer by one with zero
+    // buffering capacity.
+    // We use try_lock() instead of lock(), because someone
+    // might have leaked a StdoutLock, which would
+    // otherwise cause a deadlock here.
+    if let Some(lock) = STDOUT.try_lock() {
+        *lock.borrow_mut() = Some(LineWriter::with_capacity(0, stdout_raw()));
     }
 }
 
@@ -670,7 +663,7 @@ impl Write for Stdout {
     }
     #[inline]
     fn is_write_vectored(&self) -> bool {
-        io::Write::is_write_vectored(&&*self)
+        stdout_raw().is_write_vectored()
     }
     fn flush(&mut self) -> io::Result<()> {
         (&*self).flush()
@@ -696,7 +689,7 @@ impl Write for &Stdout {
     }
     #[inline]
     fn is_write_vectored(&self) -> bool {
-        self.lock().is_write_vectored()
+        stdout_raw().is_write_vectored()
     }
     fn flush(&mut self) -> io::Result<()> {
         self.lock().flush()
@@ -712,26 +705,37 @@ impl Write for &Stdout {
     }
 }
 
+impl StdoutLock<'_> {
+    #[inline]
+    fn with_inner<F, R>(&mut self, f: F) -> R
+    where
+        F: FnOnce(&mut LineWriter<StdoutRaw>) -> R,
+    {
+        let mut inner = self.inner.borrow_mut();
+        f(inner.get_or_insert_with(|| LineWriter::new(stdout_raw())))
+    }
+}
+
 #[stable(feature = "rust1", since = "1.0.0")]
 impl Write for StdoutLock<'_> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        self.inner.borrow_mut().write(buf)
+        self.with_inner(|stdout| stdout.write(buf))
     }
     fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result<usize> {
-        self.inner.borrow_mut().write_vectored(bufs)
+        self.with_inner(|stdout| stdout.write_vectored(bufs))
     }
     #[inline]
     fn is_write_vectored(&self) -> bool {
-        self.inner.borrow_mut().is_write_vectored()
+        stdout_raw().is_write_vectored()
     }
     fn flush(&mut self) -> io::Result<()> {
-        self.inner.borrow_mut().flush()
+        self.with_inner(|stdout| stdout.flush())
     }
     fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
-        self.inner.borrow_mut().write_all(buf)
+        self.with_inner(|stdout| stdout.write_all(buf))
     }
     fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> {
-        self.inner.borrow_mut().write_all_vectored(bufs)
+        self.with_inner(|stdout| stdout.write_all_vectored(bufs))
     }
 }
 
@@ -761,7 +765,7 @@ impl fmt::Debug for StdoutLock<'_> {
 /// standard library or via raw Windows API calls, will fail.
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct Stderr {
-    inner: Pin<&'static ReentrantMutex<RefCell<StderrRaw>>>,
+    inner: &'static ReentrantMutex<RefCell<StderrRaw>>,
 }
 
 /// A locked reference to the [`Stderr`] handle.
@@ -834,16 +838,12 @@ pub struct StderrLock<'a> {
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn stderr() -> Stderr {
     // Note that unlike `stdout()` we don't use `at_exit` here to register a
-    // destructor. Stderr is not buffered , so there's no need to run a
+    // destructor. Stderr is not buffered, so there's no need to run a
     // destructor for flushing the buffer
-    static INSTANCE: OnceLock<ReentrantMutex<RefCell<StderrRaw>>> = OnceLock::new();
+    static INSTANCE: ReentrantMutex<RefCell<StderrRaw>> =
+        ReentrantMutex::new(RefCell::new(stderr_raw()));
 
-    Stderr {
-        inner: Pin::static_ref(&INSTANCE).get_or_init_pin(
-            || unsafe { ReentrantMutex::new(RefCell::new(stderr_raw())) },
-            |mutex| unsafe { mutex.init() },
-        ),
-    }
+    Stderr { inner: &INSTANCE }
 }
 
 impl Stderr {

library/std/src/panicking.rs

@@ -18,9 +18,9 @@ use crate::intrinsics;
 use crate::mem::{self, ManuallyDrop};
 use crate::process;
 use crate::sync::atomic::{AtomicBool, Ordering};
+use crate::sync::{PoisonError, RwLock};
 use crate::sys::stdio::panic_output;
 use crate::sys_common::backtrace;
-use crate::sys_common::rwlock::StaticRwLock;
 use crate::sys_common::thread_info;
 use crate::thread;
 
@@ -71,20 +71,31 @@ extern "C" fn __rust_foreign_exception() -> ! {
     rtabort!("Rust cannot catch foreign exceptions");
 }
 
-#[derive(Copy, Clone)]
 enum Hook {
     Default,
-    Custom(*mut (dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send)),
+    Custom(Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>),
 }
 
 impl Hook {
     fn custom(f: impl Fn(&PanicInfo<'_>) + 'static + Sync + Send) -> Self {
-        Self::Custom(Box::into_raw(Box::new(f)))
+        Self::Custom(Box::new(f))
+    }
+
+    fn into_box(self) -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> {
+        match self {
+            Hook::Default => Box::new(default_hook),
+            Hook::Custom(hook) => hook,
+        }
     }
 }
 
-static HOOK_LOCK: StaticRwLock = StaticRwLock::new();
-static mut HOOK: Hook = Hook::Default;
+impl Default for Hook {
+    fn default() -> Hook {
+        Hook::Default
+    }
+}
+
+static HOOK: RwLock<Hook> = RwLock::new(Hook::Default);
 
 /// Registers a custom panic hook, replacing any that was previously registered.
 ///
@@ -125,24 +136,7 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) {
         panic!("cannot modify the panic hook from a panicking thread");
     }
 
-    // SAFETY:
-    //
-    // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`.
-    // - The argument of `Box::from_raw` is always a valid pointer that was created using
-    // `Box::into_raw`.
-    unsafe {
-        let guard = HOOK_LOCK.write();
-        let old_hook = HOOK;
-        HOOK = Hook::Custom(Box::into_raw(hook));
-        drop(guard);
-
-        if let Hook::Custom(ptr) = old_hook {
-            #[allow(unused_must_use)]
-            {
-                Box::from_raw(ptr);
-            }
-        }
-    }
+    *HOOK.write().unwrap_or_else(PoisonError::into_inner) = Hook::custom(hook);
 }
 
 /// Unregisters the current panic hook, returning it.
@@ -179,22 +173,11 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> {
         panic!("cannot modify the panic hook from a panicking thread");
     }
 
-    // SAFETY:
-    //
-    // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`.
-    // - The argument of `Box::from_raw` is always a valid pointer that was created using
-    // `Box::into_raw`.
-    unsafe {
-        let guard = HOOK_LOCK.write();
-        let hook = HOOK;
-        HOOK = Hook::Default;
-        drop(guard);
+    let mut hook = HOOK.write().unwrap_or_else(PoisonError::into_inner);
+    let old_hook = mem::take(&mut *hook);
+    drop(hook);
 
-        match hook {
-            Hook::Default => Box::new(default_hook),
-            Hook::Custom(ptr) => Box::from_raw(ptr),
-        }
-    }
+    old_hook.into_box()
 }
 
 /// Atomic combination of [`take_hook`] and [`set_hook`]. Use this to replace the panic handler with
@@ -240,24 +223,10 @@ where
         panic!("cannot modify the panic hook from a panicking thread");
     }
 
-    // SAFETY:
-    //
-    // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`.
-    // - The argument of `Box::from_raw` is always a valid pointer that was created using
-    // `Box::into_raw`.
-    unsafe {
-        let guard = HOOK_LOCK.write();
-        let old_hook = HOOK;
-        HOOK = Hook::Default;
-
-        let prev = match old_hook {
-            Hook::Default => Box::new(default_hook),
-            Hook::Custom(ptr) => Box::from_raw(ptr),
-        };
-
-        HOOK = Hook::custom(move |info| hook_fn(&prev, info));
-        drop(guard);
-    }
+    let mut hook = HOOK.write().unwrap_or_else(PoisonError::into_inner);
+    let old_hook = mem::take(&mut *hook);
+    let prev = old_hook.into_box();
+    *hook = Hook::custom(move |info| hook_fn(&prev, info));
 }
 
 fn default_hook(info: &PanicInfo<'_>) {
@@ -682,27 +651,26 @@ fn rust_panic_with_hook(
         crate::sys::abort_internal();
     }
 
-    unsafe {
-        let mut info = PanicInfo::internal_constructor(message, location, can_unwind);
-        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
-            // hook. Since string formatting happens lazily when calling `payload`
-            // methods, this means we avoid formatting the string at all!
-            // (The panic runtime might still call `payload.take_box()` though and trigger
-            // formatting.)
-            Hook::Default if panic_output().is_none() => {}
-            Hook::Default => {
-                info.set_payload(payload.get());
-                default_hook(&info);
-            }
-            Hook::Custom(ptr) => {
-                info.set_payload(payload.get());
-                (*ptr)(&info);
-            }
-        };
-    }
+    let mut info = PanicInfo::internal_constructor(message, location, can_unwind);
+    let hook = HOOK.read().unwrap_or_else(PoisonError::into_inner);
+    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
+        // hook. Since string formatting happens lazily when calling `payload`
+        // methods, this means we avoid formatting the string at all!
+        // (The panic runtime might still call `payload.take_box()` though and trigger
+        // formatting.)
+        Hook::Default if panic_output().is_none() => {}
+        Hook::Default => {
+            info.set_payload(payload.get());
+            default_hook(&info);
+        }
+        Hook::Custom(ref hook) => {
+            info.set_payload(payload.get());
+            hook(&info);
+        }
+    };
+    drop(hook);
 
     if panics > 1 || !can_unwind {
         // If a thread panics while it's already unwinding then we

library/std/src/sync/condvar.rs

@@ -3,7 +3,7 @@ mod tests;
 
 use crate::fmt;
 use crate::sync::{mutex, poison, LockResult, MutexGuard, PoisonError};
-use crate::sys_common::condvar as sys;
+use crate::sys::locks as sys;
 use crate::time::{Duration, Instant};
 
 /// A type indicating whether a timed wait on a condition variable returned