Skip to content

Commit

Permalink
Rollup merge of #97647 - m-ou-se:lazy-box-locks, r=Amanieu
Browse files Browse the repository at this point in the history
Lazily allocate and initialize pthread locks.

Lazily allocate and initialize pthread locks.

This allows {Mutex, Condvar, RwLock}::new() to be const, while still using the platform's native locks for features like priority inheritance and debug tooling. E.g. on macOS, we cannot directly use the (private) APIs that pthread's locks are implemented with, making it impossible for us to use anything other than pthread while still preserving priority inheritance, etc.

This PR doesn't yet make the public APIs const. That's for a separate PR with an FCP.

Tracking issue: #93740
  • Loading branch information
Dylan-DPC committed Jun 4, 2022
2 parents 07f586f + 6a417d4 commit e9ec022
Show file tree
Hide file tree
Showing 29 changed files with 184 additions and 123 deletions.
10 changes: 7 additions & 3 deletions library/std/src/sys/hermit/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,13 @@ impl Condvar {
mutex.lock();
res == 0
}
}

pub unsafe fn destroy(&self) {
let _ = abi::sem_destroy(self.sem1);
let _ = abi::sem_destroy(self.sem2);
impl Drop for Condvar {
fn drop(&mut self) {
unsafe {
let _ = abi::sem_destroy(self.sem1);
let _ = abi::sem_destroy(self.sem2);
}
}
}
3 changes: 0 additions & 3 deletions library/std/src/sys/hermit/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,4 @@ impl Mutex {
}
guard.locked
}

#[inline]
pub unsafe fn destroy(&self) {}
}
6 changes: 0 additions & 6 deletions library/std/src/sys/hermit/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ impl RwLock {
// FIXME: should only wake up one of these some of the time
self.cond.notify_all();
}

#[inline]
pub unsafe fn destroy(&self) {
self.lock.destroy();
self.cond.destroy();
}
}

impl State {
Expand Down
2 changes: 0 additions & 2 deletions library/std/src/sys/itron/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ impl Condvar {
unsafe { mutex.lock() };
success
}

pub unsafe fn destroy(&self) {}
}

mod waiter_queue {
Expand Down
4 changes: 3 additions & 1 deletion library/std/src/sys/itron/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ impl Mutex {
}
}
}
}

pub unsafe fn destroy(&self) {
impl Drop for Mutex {
fn drop(&mut self) {
if let Some(mtx) = self.mtx.get().map(|x| x.0) {
expect_success_aborting(unsafe { abi::del_mtx(mtx) }, &"del_mtx");
}
Expand Down
15 changes: 8 additions & 7 deletions library/std/src/sys/sgx/condvar.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::sys::locks::Mutex;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
use crate::time::Duration;

use super::waitqueue::{SpinMutex, WaitQueue, WaitVariable};
Expand All @@ -7,16 +8,19 @@ pub struct Condvar {
inner: SpinMutex<WaitVariable<()>>,
}

pub type MovableCondvar = Box<Condvar>;
pub(crate) type MovableCondvar = LazyBox<Condvar>;

impl LazyInit for Condvar {
fn init() -> Box<Self> {
Box::new(Self::new())
}
}

impl Condvar {
pub const fn new() -> Condvar {
Condvar { inner: SpinMutex::new(WaitVariable::new(())) }
}

#[inline]
pub unsafe fn init(&mut self) {}

#[inline]
pub unsafe fn notify_one(&self) {
let _ = WaitQueue::notify_one(self.inner.lock());
Expand All @@ -38,7 +42,4 @@ impl Condvar {
unsafe { mutex.lock() };
success
}

#[inline]
pub unsafe fn destroy(&self) {}
}
12 changes: 8 additions & 4 deletions library/std/src/sys/sgx/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
use super::waitqueue::{try_lock_or_false, SpinMutex, WaitQueue, WaitVariable};
use crate::sys_common::lazy_box::{LazyBox, LazyInit};

pub struct Mutex {
inner: SpinMutex<WaitVariable<bool>>,
}

// not movable: see UnsafeList implementation
pub type MovableMutex = Box<Mutex>;
pub(crate) type MovableMutex = LazyBox<Mutex>;

impl LazyInit for Mutex {
fn init() -> Box<Self> {
Box::new(Self::new())
}
}

// Implementation according to “Operating Systems: Three Easy Pieces”, chapter 28
impl Mutex {
Expand Down Expand Up @@ -52,7 +59,4 @@ impl Mutex {
true
}
}

#[inline]
pub unsafe fn destroy(&self) {}
}
12 changes: 8 additions & 4 deletions library/std/src/sys/sgx/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
mod tests;

use crate::num::NonZeroUsize;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};

use super::waitqueue::{
try_lock_or_false, NotifiedTcs, SpinMutex, SpinMutexGuard, WaitQueue, WaitVariable,
Expand All @@ -13,7 +14,13 @@ pub struct RwLock {
writer: SpinMutex<WaitVariable<bool>>,
}

pub type MovableRwLock = Box<RwLock>;
pub(crate) type MovableRwLock = LazyBox<RwLock>;

impl LazyInit for RwLock {
fn init() -> Box<Self> {
Box::new(Self::new())
}
}

// Check at compile time that RwLock size matches C definition (see test_c_rwlock_initializer below)
//
Expand Down Expand Up @@ -168,9 +175,6 @@ impl RwLock {
unsafe { self.__read_unlock(rguard, wguard) };
}
}

#[inline]
pub unsafe fn destroy(&self) {}
}

// The following functions are needed by libunwind. These symbols are named
Expand Down
4 changes: 3 additions & 1 deletion library/std/src/sys/solid/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ impl RwLock {
let rwl = self.raw();
expect_success_aborting(unsafe { abi::rwl_unl_rwl(rwl) }, &"rwl_unl_rwl");
}
}

impl Drop for RwLock {
#[inline]
pub unsafe fn destroy(&self) {
fn drop(&mut self) {
if let Some(rwl) = self.rwl.get().map(|x| x.0) {
expect_success_aborting(unsafe { abi::rwl_del_rwl(rwl) }, &"rwl_del_rwl");
}
Expand Down
9 changes: 0 additions & 9 deletions library/std/src/sys/unix/locks/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ impl Mutex {
#[inline]
pub unsafe fn init(&mut self) {}

#[inline]
pub unsafe fn destroy(&self) {}

#[inline]
pub unsafe fn try_lock(&self) -> bool {
self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok()
Expand Down Expand Up @@ -118,12 +115,6 @@ impl Condvar {
Self { futex: AtomicU32::new(0) }
}

#[inline]
pub unsafe fn init(&mut self) {}

#[inline]
pub unsafe fn destroy(&self) {}

// All the memory orderings here are `Relaxed`,
// because synchronization is done by unlocking and locking the mutex.

Expand Down
3 changes: 0 additions & 3 deletions library/std/src/sys/unix/locks/futex_rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ impl RwLock {
Self { state: AtomicU32::new(0), writer_notify: AtomicU32::new(0) }
}

#[inline]
pub unsafe fn destroy(&self) {}

#[inline]
pub unsafe fn try_read(&self) -> bool {
self.state
Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/unix/locks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ cfg_if::cfg_if! {
))] {
mod futex;
mod futex_rwlock;
pub use futex::{Mutex, MovableMutex, Condvar, MovableCondvar};
pub use futex_rwlock::{RwLock, MovableRwLock};
pub(crate) use futex::{Mutex, MovableMutex, MovableCondvar};
pub(crate) use futex_rwlock::{RwLock, MovableRwLock};
} else {
mod pthread_mutex;
mod pthread_rwlock;
mod pthread_condvar;
pub use pthread_mutex::{Mutex, MovableMutex};
pub use pthread_rwlock::{RwLock, MovableRwLock};
pub use pthread_condvar::{Condvar, MovableCondvar};
pub(crate) use pthread_mutex::{Mutex, MovableMutex};
pub(crate) use pthread_rwlock::{RwLock, MovableRwLock};
pub(crate) use pthread_condvar::MovableCondvar;
}
}
28 changes: 22 additions & 6 deletions library/std/src/sys/unix/locks/pthread_condvar.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::cell::UnsafeCell;
use crate::sys::locks::{pthread_mutex, Mutex};
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
use crate::time::Duration;

pub struct Condvar {
inner: UnsafeCell<libc::pthread_cond_t>,
}

pub type MovableCondvar = Box<Condvar>;
pub(crate) type MovableCondvar = LazyBox<Condvar>;

unsafe impl Send for Condvar {}
unsafe impl Sync for Condvar {}
Expand All @@ -18,6 +19,14 @@ fn saturating_cast_to_time_t(value: u64) -> libc::time_t {
if value > <libc::time_t>::MAX as u64 { <libc::time_t>::MAX } else { value as libc::time_t }
}

impl LazyInit for Condvar {
fn init() -> Box<Self> {
let mut condvar = Box::new(Self::new());
unsafe { condvar.init() };
condvar
}
}

impl Condvar {
pub const fn new() -> Condvar {
// Might be moved and address is changing it is better to avoid
Expand All @@ -32,14 +41,14 @@ impl Condvar {
target_os = "android",
target_os = "redox"
))]
pub unsafe fn init(&mut self) {}
unsafe fn init(&mut self) {}

// NOTE: ESP-IDF's PTHREAD_COND_INITIALIZER support is not released yet
// So on that platform, init() should always be called
// Moreover, that platform does not have pthread_condattr_setclock support,
// hence that initialization should be skipped as well
#[cfg(target_os = "espidf")]
pub unsafe fn init(&mut self) {
unsafe fn init(&mut self) {
let r = libc::pthread_cond_init(self.inner.get(), crate::ptr::null());
assert_eq!(r, 0);
}
Expand All @@ -52,7 +61,7 @@ impl Condvar {
target_os = "redox",
target_os = "espidf"
)))]
pub unsafe fn init(&mut self) {
unsafe fn init(&mut self) {
use crate::mem::MaybeUninit;
let mut attr = MaybeUninit::<libc::pthread_condattr_t>::uninit();
let r = libc::pthread_condattr_init(attr.as_mut_ptr());
Expand Down Expand Up @@ -179,14 +188,14 @@ impl Condvar {

#[inline]
#[cfg(not(target_os = "dragonfly"))]
pub unsafe fn destroy(&self) {
unsafe fn destroy(&mut self) {
let r = libc::pthread_cond_destroy(self.inner.get());
debug_assert_eq!(r, 0);
}

#[inline]
#[cfg(target_os = "dragonfly")]
pub unsafe fn destroy(&self) {
unsafe fn destroy(&mut self) {
let r = libc::pthread_cond_destroy(self.inner.get());
// On DragonFly pthread_cond_destroy() returns EINVAL if called on
// a condvar that was just initialized with
Expand All @@ -195,3 +204,10 @@ impl Condvar {
debug_assert!(r == 0 || r == libc::EINVAL);
}
}

impl Drop for Condvar {
#[inline]
fn drop(&mut self) {
unsafe { self.destroy() };
}
}
22 changes: 19 additions & 3 deletions library/std/src/sys/unix/locks/pthread_mutex.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::cell::UnsafeCell;
use crate::mem::MaybeUninit;
use crate::sys::cvt_nz;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};

pub struct Mutex {
inner: UnsafeCell<libc::pthread_mutex_t>,
}

pub type MovableMutex = Box<Mutex>;
pub(crate) type MovableMutex = LazyBox<Mutex>;

#[inline]
pub unsafe fn raw(m: &Mutex) -> *mut libc::pthread_mutex_t {
Expand All @@ -16,6 +17,14 @@ pub unsafe fn raw(m: &Mutex) -> *mut libc::pthread_mutex_t {
unsafe impl Send for Mutex {}
unsafe impl Sync for Mutex {}

impl LazyInit for Mutex {
fn init() -> Box<Self> {
let mut mutex = Box::new(Self::new());
unsafe { mutex.init() };
mutex
}
}

impl Mutex {
pub const fn new() -> Mutex {
// Might be moved to a different address, so it is better to avoid
Expand Down Expand Up @@ -73,13 +82,13 @@ impl Mutex {
}
#[inline]
#[cfg(not(target_os = "dragonfly"))]
pub unsafe fn destroy(&self) {
unsafe fn destroy(&mut self) {
let r = libc::pthread_mutex_destroy(self.inner.get());
debug_assert_eq!(r, 0);
}
#[inline]
#[cfg(target_os = "dragonfly")]
pub unsafe fn destroy(&self) {
unsafe fn destroy(&mut self) {
let r = libc::pthread_mutex_destroy(self.inner.get());
// On DragonFly pthread_mutex_destroy() returns EINVAL if called on a
// mutex that was just initialized with libc::PTHREAD_MUTEX_INITIALIZER.
Expand All @@ -89,6 +98,13 @@ impl Mutex {
}
}

impl Drop for Mutex {
#[inline]
fn drop(&mut self) {
unsafe { self.destroy() };
}
}

pub(super) struct PthreadMutexAttr<'a>(pub &'a mut MaybeUninit<libc::pthread_mutexattr_t>);

impl Drop for PthreadMutexAttr<'_> {
Expand Down
Loading

0 comments on commit e9ec022

Please sign in to comment.