Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: use sync::RwLock for internal statics #100581

Merged
merged 1 commit into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 50 additions & 79 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -71,20 +71,29 @@ 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)))
#[inline]
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 {
#[inline]
fn default() -> Hook {
Hook::Default
}
}

static HOOK: RwLock<Hook> = RwLock::new(Hook::Default);

/// Registers a custom panic hook, replacing any that was previously registered.
///
Expand Down Expand Up @@ -125,24 +134,13 @@ 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);
}
}
}
let new = Hook::Custom(hook);
let mut hook = HOOK.write().unwrap_or_else(PoisonError::into_inner);
let old = mem::replace(&mut *hook, new);
drop(hook);
// Only drop the old hook after releasing the lock to avoid deadlocking
// if its destructor panics.
drop(old);
}

/// Unregisters the current panic hook, returning it.
Expand Down Expand Up @@ -179,22 +177,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
Expand Down Expand Up @@ -240,24 +227,9 @@ 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 prev = mem::take(&mut *hook).into_box();
*hook = Hook::Custom(Box::new(move |info| hook_fn(&prev, info)));
}

fn default_hook(info: &PanicInfo<'_>) {
Expand Down Expand Up @@ -682,27 +654,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
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/solid/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::os::{
solid::ffi::{OsStrExt, OsStringExt},
};
use crate::path::{self, PathBuf};
use crate::sys_common::rwlock::StaticRwLock;
use crate::sync::RwLock;
use crate::vec;

use super::{error, itron, memchr};
Expand Down Expand Up @@ -78,7 +78,7 @@ pub fn current_exe() -> io::Result<PathBuf> {
unsupported()
}

static ENV_LOCK: StaticRwLock = StaticRwLock::new();
static ENV_LOCK: RwLock<()> = RwLock::new(());

pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/unix/locks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@ cfg_if::cfg_if! {
mod futex_rwlock;
mod futex_condvar;
pub(crate) use futex_mutex::{Mutex, MovableMutex};
pub(crate) use futex_rwlock::{RwLock, MovableRwLock};
pub(crate) use futex_rwlock::MovableRwLock;
pub(crate) use futex_condvar::MovableCondvar;
} else if #[cfg(target_os = "fuchsia")] {
mod fuchsia_mutex;
mod futex_rwlock;
mod futex_condvar;
pub(crate) use fuchsia_mutex::{Mutex, MovableMutex};
pub(crate) use futex_rwlock::{RwLock, MovableRwLock};
pub(crate) use futex_rwlock::MovableRwLock;
pub(crate) use futex_condvar::MovableCondvar;
} else {
mod pthread_mutex;
mod pthread_rwlock;
mod pthread_condvar;
pub(crate) use pthread_mutex::{Mutex, MovableMutex};
pub(crate) use pthread_rwlock::{RwLock, MovableRwLock};
pub(crate) use pthread_rwlock::MovableRwLock;
pub(crate) use pthread_condvar::MovableCondvar;
}
}
8 changes: 4 additions & 4 deletions library/std/src/sys/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ use crate::path::{self, PathBuf};
use crate::ptr;
use crate::slice;
use crate::str;
use crate::sync::{PoisonError, RwLock};
use crate::sys::cvt;
use crate::sys::fd;
use crate::sys::memchr;
use crate::sys_common::rwlock::{StaticRwLock, StaticRwLockReadGuard};
use crate::vec;

#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))]
Expand Down Expand Up @@ -501,10 +501,10 @@ pub unsafe fn environ() -> *mut *const *const c_char {
ptr::addr_of_mut!(environ)
}

static ENV_LOCK: StaticRwLock = StaticRwLock::new();
static ENV_LOCK: RwLock<()> = RwLock::new(());

pub fn env_read_lock() -> StaticRwLockReadGuard {
ENV_LOCK.read()
pub fn env_read_lock() -> impl Drop {
ENV_LOCK.read().unwrap_or_else(PoisonError::into_inner)
}

/// Returns a vector of (variable, value) byte-vector pairs for all the
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unsupported/locks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ mod mutex;
mod rwlock;
pub use condvar::{Condvar, MovableCondvar};
pub use mutex::{MovableMutex, Mutex};
pub use rwlock::{MovableRwLock, RwLock};
pub use rwlock::MovableRwLock;
2 changes: 1 addition & 1 deletion library/std/src/sys/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ cfg_if::cfg_if! {
mod futex_rwlock;
pub(crate) use futex_condvar::{Condvar, MovableCondvar};
pub(crate) use futex_mutex::{Mutex, MovableMutex};
pub(crate) use futex_rwlock::{RwLock, MovableRwLock};
pub(crate) use futex_rwlock::MovableRwLock;
}
#[path = "atomics/futex.rs"]
pub mod futex;
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/locks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ mod mutex;
mod rwlock;
pub use condvar::{Condvar, MovableCondvar};
pub use mutex::{MovableMutex, Mutex};
pub use rwlock::{MovableRwLock, RwLock};
pub use rwlock::MovableRwLock;
60 changes: 0 additions & 60 deletions library/std/src/sys_common/rwlock.rs
Original file line number Diff line number Diff line change
@@ -1,65 +1,5 @@
use crate::sys::locks as imp;

/// An OS-based reader-writer lock, meant for use in static variables.
///
/// This rwlock does not implement poisoning.
///
/// This rwlock has a const constructor ([`StaticRwLock::new`]), does not
/// implement `Drop` to cleanup resources.
pub struct StaticRwLock(imp::RwLock);

impl StaticRwLock {
/// Creates a new rwlock for use.
#[inline]
pub const fn new() -> Self {
Self(imp::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(&'static self) -> StaticRwLockReadGuard {
unsafe { self.0.read() };
StaticRwLockReadGuard(&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(&'static self) -> StaticRwLockWriteGuard {
unsafe { self.0.write() };
StaticRwLockWriteGuard(&self.0)
}
}

#[must_use]
pub struct StaticRwLockReadGuard(&'static imp::RwLock);

impl Drop for StaticRwLockReadGuard {
#[inline]
fn drop(&mut self) {
unsafe {
self.0.read_unlock();
}
}
}

#[must_use]
pub struct StaticRwLockWriteGuard(&'static imp::RwLock);

impl Drop for StaticRwLockWriteGuard {
#[inline]
fn drop(&mut self) {
unsafe {
self.0.write_unlock();
}
}
}

/// An OS-based reader-writer lock.
///
/// This rwlock cleans up its resources in its `Drop` implementation and may
Expand Down