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

Multiple improvements to RwLocks #84687

Merged
merged 1 commit into from
Jun 10, 2021
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
15 changes: 7 additions & 8 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
m-ou-se marked this conversation as resolved.
Show resolved Hide resolved
static mut HOOK: Hook = Hook::Default;

/// Registers a custom panic hook, replacing any that was previously registered.
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -626,7 +626,6 @@ fn rust_panic_with_hook(
(*ptr)(&info);
}
};
HOOK_LOCK.read_unlock();
}

if panics > 1 {
Expand Down
34 changes: 4 additions & 30 deletions library/std/src/sync/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/hermit/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ pub struct RWLock {
state: UnsafeCell<State>,
}

pub type MovableRWLock = Box<RWLock>;

enum State {
Unlocked,
Reading(usize),
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/sgx/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions library/std/src/sys/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down
54 changes: 2 additions & 52 deletions library/std/src/sys/unix/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down Expand Up @@ -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() }
}
}
2 changes: 2 additions & 0 deletions library/std/src/sys/unsupported/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/wasm/atomics/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ pub struct RWLock {
state: UnsafeCell<State>,
}

pub type MovableRWLock = RWLock;

enum State {
Unlocked,
Reading(usize),
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/windows/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down
Loading