Skip to content

Commit

Permalink
Auto merge of #56410 - faern:add-parking-lot, r=<try>
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Apr 3, 2019
2 parents f8673e0 + e0d5bc1 commit 8f8fabb
Show file tree
Hide file tree
Showing 63 changed files with 561 additions and 4,023 deletions.
4 changes: 4 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/libstd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -73,6 +73,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
8 changes: 3 additions & 5 deletions src/libstd/io/lazy.rs
Original file line number Diff line number Diff line change
@@ -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>>,
}

Expand All @@ -18,7 +16,7 @@ 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()),
}
}
Expand Down
34 changes: 28 additions & 6 deletions src/libstd/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use crate::cell::RefCell;
use crate::fmt;
use crate::io::lazy::Lazy;
use crate::io::{self, Initializer, BufReader, LineWriter};
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! {
Expand Down Expand Up @@ -256,7 +257,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.
Expand Down Expand Up @@ -467,7 +468,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() }
}
}

Expand All @@ -493,6 +494,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> {
Expand All @@ -510,6 +517,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.
Expand Down Expand Up @@ -620,7 +632,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() }
}
}

Expand All @@ -646,6 +658,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> {
Expand All @@ -663,6 +681,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
Expand Down Expand Up @@ -767,7 +790,6 @@ pub use realstd::io::{_eprint, _print};

#[cfg(test)]
mod tests {
use crate::panic::{UnwindSafe, RefUnwindSafe};
use crate::thread;
use super::*;

Expand Down
12 changes: 12 additions & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
17 changes: 9 additions & 8 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 8f8fabb

Please sign in to comment.