Skip to content

Commit

Permalink
std: replace LazyBox with OnceBox
Browse files Browse the repository at this point in the history
This PR replaces the `LazyBox` wrapper used to allocate the pthread primitives with `OnceBox`, which has a more familiar API mirroring that of `OnceLock`. This cleans up the code in preparation for larger changes like rust-lang#128184 (from which this PR was split) and allows some neat optimizations, like avoid an acquire-load of the allocation pointer in `Mutex::unlock`, where the initialization of the allocation must have already been observed.

Additionally, I've gotten rid of the TEEOS `Condvar` code, it's just a duplicate of the pthread one anyway and I didn't want to repeat myself.
  • Loading branch information
joboet committed Oct 1, 2024
1 parent 21aa500 commit c1acccd
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 284 deletions.
8 changes: 4 additions & 4 deletions library/std/src/sys/sync/condvar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ cfg_if::cfg_if! {
))] {
mod futex;
pub use futex::Condvar;
} else if #[cfg(target_family = "unix")] {
} else if #[cfg(any(
target_family = "unix",
target_os = "teeos",
))] {
mod pthread;
pub use pthread::Condvar;
} else if #[cfg(all(target_os = "windows", target_vendor = "win7"))] {
Expand All @@ -24,9 +27,6 @@ cfg_if::cfg_if! {
} else if #[cfg(target_os = "solid_asp3")] {
mod itron;
pub use itron::Condvar;
} else if #[cfg(target_os = "teeos")] {
mod teeos;
pub use teeos::Condvar;
} else if #[cfg(target_os = "xous")] {
mod xous;
pub use xous::Condvar;
Expand Down
38 changes: 18 additions & 20 deletions library/std/src/sys/sync/condvar/pthread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,25 @@ use crate::cell::UnsafeCell;
use crate::ptr;
use crate::sync::atomic::AtomicPtr;
use crate::sync::atomic::Ordering::Relaxed;
use crate::sys::sync::{Mutex, mutex};
use crate::sys::sync::{Mutex, OnceBox};
#[cfg(not(target_os = "nto"))]
use crate::sys::time::TIMESPEC_MAX;
#[cfg(target_os = "nto")]
use crate::sys::time::TIMESPEC_MAX_CAPPED;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
use crate::time::Duration;

struct AllocatedCondvar(UnsafeCell<libc::pthread_cond_t>);

pub struct Condvar {
inner: LazyBox<AllocatedCondvar>,
inner: OnceBox<AllocatedCondvar>,
mutex: AtomicPtr<libc::pthread_mutex_t>,
}

#[inline]
fn raw(c: &Condvar) -> *mut libc::pthread_cond_t {
c.inner.0.get()
}

unsafe impl Send for AllocatedCondvar {}
unsafe impl Sync for AllocatedCondvar {}

impl LazyInit for AllocatedCondvar {
fn init() -> Box<Self> {
impl AllocatedCondvar {
fn new() -> Box<Self> {
let condvar = Box::new(AllocatedCondvar(UnsafeCell::new(libc::PTHREAD_COND_INITIALIZER)));

cfg_if::cfg_if! {
Expand All @@ -37,7 +31,7 @@ impl LazyInit for AllocatedCondvar {
target_vendor = "apple",
))] {
// `pthread_condattr_setclock` is unfortunately not supported on these platforms.
} else if #[cfg(any(target_os = "espidf", target_os = "horizon"))] {
} else if #[cfg(any(target_os = "espidf", target_os = "horizon", target_os = "teeos"))] {
// 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,
Expand Down Expand Up @@ -82,7 +76,11 @@ impl Drop for AllocatedCondvar {

impl Condvar {
pub const fn new() -> Condvar {
Condvar { inner: LazyBox::new(), mutex: AtomicPtr::new(ptr::null_mut()) }
Condvar { inner: OnceBox::new(), mutex: AtomicPtr::new(ptr::null_mut()) }
}

fn get(&self) -> *mut libc::pthread_cond_t {
self.inner.get_or_init(AllocatedCondvar::new).0.get()
}

#[inline]
Expand All @@ -98,21 +96,21 @@ impl Condvar {

#[inline]
pub fn notify_one(&self) {
let r = unsafe { libc::pthread_cond_signal(raw(self)) };
let r = unsafe { libc::pthread_cond_signal(self.get()) };
debug_assert_eq!(r, 0);
}

#[inline]
pub fn notify_all(&self) {
let r = unsafe { libc::pthread_cond_broadcast(raw(self)) };
let r = unsafe { libc::pthread_cond_broadcast(self.get()) };
debug_assert_eq!(r, 0);
}

#[inline]
pub unsafe fn wait(&self, mutex: &Mutex) {
let mutex = mutex::raw(mutex);
let mutex = mutex.get_assert_locked();
self.verify(mutex);
let r = libc::pthread_cond_wait(raw(self), mutex);
let r = libc::pthread_cond_wait(self.get(), mutex);
debug_assert_eq!(r, 0);
}

Expand All @@ -129,7 +127,7 @@ impl Condvar {
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
use crate::sys::time::Timespec;

let mutex = mutex::raw(mutex);
let mutex = mutex.get_assert_locked();
self.verify(mutex);

#[cfg(not(target_os = "nto"))]
Expand All @@ -144,7 +142,7 @@ impl Condvar {
.and_then(|t| t.to_timespec_capped())
.unwrap_or(TIMESPEC_MAX_CAPPED);

let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
let r = libc::pthread_cond_timedwait(self.get(), mutex, &timeout);
assert!(r == libc::ETIMEDOUT || r == 0);
r == 0
}
Expand All @@ -162,7 +160,7 @@ impl Condvar {
use crate::sys::time::SystemTime;
use crate::time::Instant;

let mutex = mutex::raw(mutex);
let mutex = mutex.get_assert_locked();
self.verify(mutex);

// OSX implementation of `pthread_cond_timedwait` is buggy
Expand All @@ -188,7 +186,7 @@ impl Condvar {
.and_then(|t| t.to_timespec())
.unwrap_or(TIMESPEC_MAX);

let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
let r = libc::pthread_cond_timedwait(self.get(), mutex, &timeout);
debug_assert!(r == libc::ETIMEDOUT || r == 0);

// ETIMEDOUT is not a totally reliable method of determining timeout due
Expand Down
29 changes: 12 additions & 17 deletions library/std/src/sys/sync/condvar/sgx.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,39 @@
use crate::sys::pal::waitqueue::{SpinMutex, WaitQueue, WaitVariable};
use crate::sys::sync::Mutex;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
use crate::sys::sync::{Mutex, OnceBox};
use crate::time::Duration;

/// FIXME: `UnsafeList` is not movable.
struct AllocatedCondvar(SpinMutex<WaitVariable<()>>);

pub struct Condvar {
inner: LazyBox<AllocatedCondvar>,
}

impl LazyInit for AllocatedCondvar {
fn init() -> Box<Self> {
Box::new(AllocatedCondvar(SpinMutex::new(WaitVariable::new(()))))
}
// FIXME: `UnsafeList` is not movable.
inner: OnceBox<SpinMutex<WaitVariable<()>>>,
}

impl Condvar {
pub const fn new() -> Condvar {
Condvar { inner: LazyBox::new() }
Condvar { inner: OnceBox::new() }
}

fn get(&self) -> &SpinMutex<WaitVariable<()>> {
self.inner.get_or_init(|| Box::new(SpinMutex::new(WaitVariable::new(()))))
}

#[inline]
pub fn notify_one(&self) {
let _ = WaitQueue::notify_one(self.inner.0.lock());
let _ = WaitQueue::notify_one(self.get().lock());
}

#[inline]
pub fn notify_all(&self) {
let _ = WaitQueue::notify_all(self.inner.0.lock());
let _ = WaitQueue::notify_all(self.get().lock());
}

pub unsafe fn wait(&self, mutex: &Mutex) {
let guard = self.inner.0.lock();
let guard = self.get().lock();
WaitQueue::wait(guard, || unsafe { mutex.unlock() });
mutex.lock()
}

pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
let success = WaitQueue::wait_timeout(&self.inner.0, dur, || unsafe { mutex.unlock() });
let success = WaitQueue::wait_timeout(self.get(), dur, || unsafe { mutex.unlock() });
mutex.lock();
success
}
Expand Down
101 changes: 0 additions & 101 deletions library/std/src/sys/sync/condvar/teeos.rs

This file was deleted.

3 changes: 3 additions & 0 deletions library/std/src/sys/sync/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
mod condvar;
mod mutex;
mod once;
mod once_box;
mod rwlock;
mod thread_parking;

pub use condvar::Condvar;
pub use mutex::Mutex;
pub use once::{Once, OnceState};
#[allow(unused)] // Only used on some platforms.
use once_box::OnceBox;
pub use rwlock::RwLock;
pub use thread_parking::Parker;
2 changes: 1 addition & 1 deletion library/std/src/sys/sync/mutex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ cfg_if::cfg_if! {
target_os = "teeos",
))] {
mod pthread;
pub use pthread::{Mutex, raw};
pub use pthread::Mutex;
} else if #[cfg(all(target_os = "windows", target_vendor = "win7"))] {
mod windows7;
pub use windows7::{Mutex, raw};
Expand Down
Loading

0 comments on commit c1acccd

Please sign in to comment.