Auto merge of #56410 - faern:add-parking-lot, r=<try>

Use the parking_lot locking primitives

This PR adds the [`parking_lot`](https://crates.io/crates/parking_lot) code to libstd and uses it for the `sys_common::{mutex,rwlock,condvar,remutex}` implementations.

This has been discussed in https://internals.rust-lang.org/t/standard-library-synchronization-primitives-and-undefined-behavior/8439/9

Thanks @Amanieu for mentoring when doing this, and basically all the code is his as well of course.

Fixes #35836
Fixes #53127

Author: bors

.gitmodules

@@ -47,3 +47,7 @@
 [submodule "src/doc/embedded-book"]
 	path = src/doc/embedded-book
 	url = https://github.com/rust-embedded/book.git
+[submodule "src/parking_lot"]
+	path = src/parking_lot
+	url = https://github.com/faern/parking_lot
+

Cargo.toml

@@ -28,6 +28,7 @@ exclude = [
   "build",
   # HACK(eddyb) This hardcodes the fact that our CI uses `/checkout/obj`.
   "obj",
+  "src/parking_lot/benchmark",
 ]
 
 # Curiously, LLVM 7.0 will segfault if compiled with opt-level=3

src/ci/docker/dist-various-2/Dockerfile

@@ -32,7 +32,7 @@ RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc
 COPY dist-various-2/build-x86_64-fortanix-unknown-sgx-toolchain.sh /tmp/
 # We pass the commit id of the port of LLVM's libunwind to the build script.
 # Any update to the commit id here, should cause the container image to be re-built from this point on.
-RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh "53b586346f2c7870e20b170decdc30729d97c42b"
+RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh "a50a70f1394b2e62d6a5d2510330eb110e31dad4"
 
 COPY dist-various-2/build-wasi-toolchain.sh /tmp/
 RUN /tmp/build-wasi-toolchain.sh

src/libstd/Cargo.toml

@@ -48,7 +48,7 @@ fortanix-sgx-abi = { version = "0.3.2", features = ['rustc-dep-of-std'] }
 cc = "1.0"
 
 [features]
-default = ["compiler_builtins_c", "std_detect_file_io", "std_detect_dlsym_getauxval"]
+default = ["compiler_builtins_c", "std_detect_file_io", "std_detect_dlsym_getauxval", "i-am-libstd"]
 
 backtrace = ["backtrace-sys"]
 panic-unwind = ["panic_unwind"]
@@ -74,6 +74,9 @@ wasm-bindgen-threads = []
 std_detect_file_io = []
 std_detect_dlsym_getauxval = []
 
+# Feature used by parking_lot
+i-am-libstd = []
+
 [package.metadata.fortanix-sgx]
 # Maximum possible number of threads when testing
 threads = 125

src/libstd/io/lazy.rs

@@ -1,12 +1,10 @@
 use crate::cell::Cell;
 use crate::ptr;
-use crate::sync::Arc;
+use crate::sync::{Arc, RawMutex};
 use crate::sys_common;
-use crate::sys_common::mutex::Mutex;
 
 pub struct Lazy<T> {
-    // We never call `lock.init()`, so it is UB to attempt to acquire this mutex reentrantly!
-    lock: Mutex,
+    lock: RawMutex,
     ptr: Cell<*mut Arc<T>>,
 }
 
@@ -18,24 +16,24 @@ unsafe impl<T> Sync for Lazy<T> {}
 impl<T> Lazy<T> {
     pub const fn new() -> Lazy<T> {
         Lazy {
-            lock: Mutex::new(),
+            lock: RawMutex::new(),
             ptr: Cell::new(ptr::null_mut()),
         }
     }
 }
 
 impl<T: Send + Sync + 'static> Lazy<T> {
-    /// Safety: `init` must not call `get` on the variable that is being
-    /// initialized.
-    pub unsafe fn get(&'static self, init: fn() -> Arc<T>) -> Option<Arc<T>> {
+    /// Warning: `init` must not call `get` on the variable that is being
+    /// initialized. Doing so will cause a deadlock.
+    pub fn get(&'static self, init: fn() -> Arc<T>) -> Option<Arc<T>> {
         let _guard = self.lock.lock();
         let ptr = self.ptr.get();
         if ptr.is_null() {
-            Some(self.init(init))
+            Some(unsafe { self.init(init) })
         } else if ptr == done() {
             None
         } else {
-            Some((*ptr).clone())
+            Some(unsafe { (*ptr).clone() })
         }
     }
 

src/libstd/io/stdio.rs

@@ -6,9 +6,10 @@ use crate::cell::RefCell;
 use crate::fmt;
 use crate::io::lazy::Lazy;
 use crate::io::{self, Initializer, BufReader, LineWriter, IoVec, IoVecMut};
-use crate::sync::{Arc, Mutex, MutexGuard};
+use crate::sync::Arc;
 use crate::sys::stdio;
-use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
+use crate::panic::{UnwindSafe, RefUnwindSafe};
+use crate::parking_lot::{Mutex, MutexGuard, ReentrantMutex, ReentrantMutexGuard};
 use crate::thread::LocalKey;
 
 thread_local! {
@@ -242,9 +243,7 @@ pub struct StdinLock<'a> {
 pub fn stdin() -> Stdin {
     static INSTANCE: Lazy<Mutex<BufReader<Maybe<StdinRaw>>>> = Lazy::new();
     return Stdin {
-        inner: unsafe {
-            INSTANCE.get(stdin_init).expect("cannot access stdin during shutdown")
-        },
+        inner: INSTANCE.get(stdin_init).expect("cannot access stdin during shutdown"),
     };
 
     fn stdin_init() -> Arc<Mutex<BufReader<Maybe<StdinRaw>>>> {
@@ -285,7 +284,7 @@ impl Stdin {
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn lock(&self) -> StdinLock<'_> {
-        StdinLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
+        StdinLock { inner: self.inner.lock() }
     }
 
     /// Locks this handle and reads a line of input into the specified buffer.
@@ -466,9 +465,7 @@ pub struct StdoutLock<'a> {
 pub fn stdout() -> Stdout {
     static INSTANCE: Lazy<ReentrantMutex<RefCell<LineWriter<Maybe<StdoutRaw>>>>> = Lazy::new();
     return Stdout {
-        inner: unsafe {
-            INSTANCE.get(stdout_init).expect("cannot access stdout during shutdown")
-        },
+        inner: INSTANCE.get(stdout_init).expect("cannot access stdout during shutdown"),
     };
 
     fn stdout_init() -> Arc<ReentrantMutex<RefCell<LineWriter<Maybe<StdoutRaw>>>>> {
@@ -504,7 +501,7 @@ impl Stdout {
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn lock(&self) -> StdoutLock<'_> {
-        StdoutLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
+        StdoutLock { inner: self.inner.lock() }
     }
 }
 
@@ -533,6 +530,12 @@ impl Write for Stdout {
         self.lock().write_fmt(args)
     }
 }
+
+#[stable(feature = "rust1", since = "1.0.0")]
+impl UnwindSafe for Stdout {}
+#[stable(feature = "rust1", since = "1.0.0")]
+impl RefUnwindSafe for Stdout {}
+
 #[stable(feature = "rust1", since = "1.0.0")]
 impl Write for StdoutLock<'_> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
@@ -553,6 +556,11 @@ impl fmt::Debug for StdoutLock<'_> {
     }
 }
 
+#[stable(feature = "rust1", since = "1.0.0")]
+impl UnwindSafe for StdoutLock<'_> {}
+#[stable(feature = "rust1", since = "1.0.0")]
+impl RefUnwindSafe for StdoutLock<'_> {}
+
 /// A handle to the standard error stream of a process.
 ///
 /// For more information, see the [`io::stderr`] method.
@@ -625,9 +633,7 @@ pub struct StderrLock<'a> {
 pub fn stderr() -> Stderr {
     static INSTANCE: Lazy<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> = Lazy::new();
     return Stderr {
-        inner: unsafe {
-            INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown")
-        },
+        inner: INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown"),
     };
 
     fn stderr_init() -> Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> {
@@ -663,7 +669,7 @@ impl Stderr {
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn lock(&self) -> StderrLock<'_> {
-        StderrLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
+        StderrLock { inner: self.inner.lock() }
     }
 }
 
@@ -692,6 +698,12 @@ impl Write for Stderr {
         self.lock().write_fmt(args)
     }
 }
+
+#[stable(feature = "rust1", since = "1.0.0")]
+impl UnwindSafe for Stderr {}
+#[stable(feature = "rust1", since = "1.0.0")]
+impl RefUnwindSafe for Stderr {}
+
 #[stable(feature = "rust1", since = "1.0.0")]
 impl Write for StderrLock<'_> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
@@ -712,6 +724,11 @@ impl fmt::Debug for StderrLock<'_> {
     }
 }
 
+#[stable(feature = "rust1", since = "1.0.0")]
+impl UnwindSafe for StderrLock<'_> {}
+#[stable(feature = "rust1", since = "1.0.0")]
+impl RefUnwindSafe for StderrLock<'_> {}
+
 /// Resets the thread-local stderr handle to the specified writer
 ///
 /// This will replace the current thread's stderr handle, returning the old
@@ -816,7 +833,6 @@ pub use realstd::io::{_eprint, _print};
 
 #[cfg(test)]
 mod tests {
-    use crate::panic::{UnwindSafe, RefUnwindSafe};
     use crate::thread;
     use super::*;
 

src/libstd/lib.rs

@@ -499,6 +499,18 @@ pub mod rt;
 #[cfg(not(test))]
 mod std_detect;
 
+#[path = "../parking_lot/core/src/lib.rs"]
+mod parking_lot_core;
+
+#[path = "../parking_lot/lock_api/src/lib.rs"]
+#[allow(dead_code)]
+mod lock_api;
+
+#[path = "../parking_lot/src/lib.rs"]
+#[allow(dead_code)]
+mod parking_lot;
+
+
 #[doc(hidden)]
 #[unstable(feature = "stdsimd", issue = "48556")]
 #[cfg(not(test))]

src/libstd/panicking.rs

@@ -12,11 +12,12 @@ use core::panic::{BoxMeUp, PanicInfo, Location};
 use crate::any::Any;
 use crate::fmt;
 use crate::intrinsics;
+use crate::lock_api::RawRwLock as _;
 use crate::mem;
+use crate::parking_lot::RawRwLock;
 use crate::ptr;
 use crate::raw;
 use crate::sys::stdio::panic_output;
-use crate::sys_common::rwlock::RWLock;
 use crate::sys_common::thread_info;
 use crate::sys_common::util;
 use crate::thread;
@@ -54,7 +55,7 @@ enum Hook {
     Custom(*mut (dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send)),
 }
 
-static HOOK_LOCK: RWLock = RWLock::new();
+static HOOK_LOCK: RawRwLock = RawRwLock::INIT;
 static mut HOOK: Hook = Hook::Default;
 
 /// Registers a custom panic hook, replacing any that was previously registered.
@@ -97,10 +98,10 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) {
     }
 
     unsafe {
-        HOOK_LOCK.write();
+        HOOK_LOCK.lock_exclusive();
         let old_hook = HOOK;
         HOOK = Hook::Custom(Box::into_raw(hook));
-        HOOK_LOCK.write_unlock();
+        HOOK_LOCK.unlock_exclusive();
 
         if let Hook::Custom(ptr) = old_hook {
             Box::from_raw(ptr);
@@ -142,10 +143,10 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> {
     }
 
     unsafe {
-        HOOK_LOCK.write();
+        HOOK_LOCK.lock_exclusive();
         let hook = HOOK;
         HOOK = Hook::Default;
-        HOOK_LOCK.write_unlock();
+        HOOK_LOCK.unlock_exclusive();
 
         match hook {
             Hook::Default => Box::new(default_hook),
@@ -463,7 +464,7 @@ fn rust_panic_with_hook(payload: &mut dyn BoxMeUp,
             message,
             Location::internal_constructor(file, line, col),
         );
-        HOOK_LOCK.read();
+        HOOK_LOCK.lock_shared();
         match HOOK {
             // Some platforms know that printing to stderr won't ever actually
             // print anything, and if that's the case we can skip the default
@@ -478,7 +479,7 @@ fn rust_panic_with_hook(payload: &mut dyn BoxMeUp,
                 (*ptr)(&info);
             }
         };
-        HOOK_LOCK.read_unlock();
+        HOOK_LOCK.unlock_shared();
     }
 
     if panics > 1 {