std: make ReentrantMutex movable and const; simplify Stdout initialization

This commit is contained in:
joboet 2022-09-03 14:05:28 +02:00
parent 75b7e52e92
commit 8c37fdf2d7
No known key found for this signature in database
GPG Key ID: 704E0149B0194B3C
12 changed files with 39 additions and 165 deletions

View File

@ -8,7 +8,6 @@ use crate::io::prelude::*;
use crate::cell::{Cell, RefCell}; use crate::cell::{Cell, RefCell};
use crate::fmt; use crate::fmt;
use crate::io::{self, BufReader, IoSlice, IoSliceMut, LineWriter, Lines}; use crate::io::{self, BufReader, IoSlice, IoSliceMut, LineWriter, Lines};
use crate::pin::Pin;
use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sync::atomic::{AtomicBool, Ordering};
use crate::sync::{Arc, Mutex, MutexGuard, OnceLock}; use crate::sync::{Arc, Mutex, MutexGuard, OnceLock};
use crate::sys::stdio; use crate::sys::stdio;
@ -526,7 +525,7 @@ pub struct Stdout {
// FIXME: this should be LineWriter or BufWriter depending on the state of // FIXME: this should be LineWriter or BufWriter depending on the state of
// stdout (tty or not). Note that if this is not line buffered it // stdout (tty or not). Note that if this is not line buffered it
// should also flush-on-panic or some form of flush-on-abort. // should also flush-on-panic or some form of flush-on-abort.
inner: Pin<&'static ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>>, inner: &'static ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>,
} }
/// A locked reference to the [`Stdout`] handle. /// A locked reference to the [`Stdout`] handle.
@ -603,24 +602,20 @@ static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLo
#[stable(feature = "rust1", since = "1.0.0")] #[stable(feature = "rust1", since = "1.0.0")]
pub fn stdout() -> Stdout { pub fn stdout() -> Stdout {
Stdout { Stdout {
inner: Pin::static_ref(&STDOUT).get_or_init_pin( inner: STDOUT
|| unsafe { ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))) }, .get_or_init(|| ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw())))),
|mutex| unsafe { mutex.init() },
),
} }
} }
pub fn cleanup() { pub fn cleanup() {
if let Some(instance) = STDOUT.get() { // Flush the data and disable buffering during shutdown
// Flush the data and disable buffering during shutdown // by replacing the line writer by one with zero
// by replacing the line writer by one with zero // buffering capacity.
// buffering capacity. // We use try_lock() instead of lock(), because someone
// We use try_lock() instead of lock(), because someone // might have leaked a StdoutLock, which would
// might have leaked a StdoutLock, which would // otherwise cause a deadlock here.
// otherwise cause a deadlock here. if let Some(lock) = STDOUT.get().and_then(ReentrantMutex::try_lock) {
if let Some(lock) = Pin::static_ref(instance).try_lock() { *lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
*lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
}
} }
} }
@ -761,7 +756,7 @@ impl fmt::Debug for StdoutLock<'_> {
/// standard library or via raw Windows API calls, will fail. /// standard library or via raw Windows API calls, will fail.
#[stable(feature = "rust1", since = "1.0.0")] #[stable(feature = "rust1", since = "1.0.0")]
pub struct Stderr { pub struct Stderr {
inner: Pin<&'static ReentrantMutex<RefCell<StderrRaw>>>, inner: &'static ReentrantMutex<RefCell<StderrRaw>>,
} }
/// A locked reference to the [`Stderr`] handle. /// A locked reference to the [`Stderr`] handle.
@ -834,16 +829,12 @@ pub struct StderrLock<'a> {
#[stable(feature = "rust1", since = "1.0.0")] #[stable(feature = "rust1", since = "1.0.0")]
pub fn stderr() -> Stderr { pub fn stderr() -> Stderr {
// Note that unlike `stdout()` we don't use `at_exit` here to register a // Note that unlike `stdout()` we don't use `at_exit` here to register a
// destructor. Stderr is not buffered , so there's no need to run a // destructor. Stderr is not buffered, so there's no need to run a
// destructor for flushing the buffer // destructor for flushing the buffer
static INSTANCE: OnceLock<ReentrantMutex<RefCell<StderrRaw>>> = OnceLock::new(); static INSTANCE: ReentrantMutex<RefCell<StderrRaw>> =
ReentrantMutex::new(RefCell::new(stderr_raw()));
Stderr { Stderr { inner: &INSTANCE }
inner: Pin::static_ref(&INSTANCE).get_or_init_pin(
|| unsafe { ReentrantMutex::new(RefCell::new(stderr_raw())) },
|mutex| unsafe { mutex.init() },
),
}
} }
impl Stderr { impl Stderr {

View File

@ -3,7 +3,6 @@ use crate::fmt;
use crate::marker::PhantomData; use crate::marker::PhantomData;
use crate::mem::MaybeUninit; use crate::mem::MaybeUninit;
use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::panic::{RefUnwindSafe, UnwindSafe};
use crate::pin::Pin;
use crate::sync::Once; use crate::sync::Once;
/// A synchronization primitive which can be written to only once. /// A synchronization primitive which can be written to only once.
@ -223,60 +222,6 @@ impl<T> OnceLock<T> {
Ok(unsafe { self.get_unchecked() }) Ok(unsafe { self.get_unchecked() })
} }
/// Internal-only API that gets the contents of the cell, initializing it
/// in two steps with `f` and `g` if the cell was empty.
///
/// `f` is called to construct the value, which is then moved into the cell
/// and given as a (pinned) mutable reference to `g` to finish
/// initialization.
///
/// This allows `g` to inspect an manipulate the value after it has been
/// moved into its final place in the cell, but before the cell is
/// considered initialized.
///
/// # Panics
///
/// If `f` or `g` panics, the panic is propagated to the caller, and the
/// cell remains uninitialized.
///
/// With the current implementation, if `g` panics, the value from `f` will
/// not be dropped. This should probably be fixed if this is ever used for
/// a type where this matters.
///
/// It is an error to reentrantly initialize the cell from `f`. The exact
/// outcome is unspecified. Current implementation deadlocks, but this may
/// be changed to a panic in the future.
pub(crate) fn get_or_init_pin<F, G>(self: Pin<&Self>, f: F, g: G) -> Pin<&T>
where
F: FnOnce() -> T,
G: FnOnce(Pin<&mut T>),
{
if let Some(value) = self.get_ref().get() {
// SAFETY: The inner value was already initialized, and will not be
// moved anymore.
return unsafe { Pin::new_unchecked(value) };
}
let slot = &self.value;
// Ignore poisoning from other threads
// If another thread panics, then we'll be able to run our closure
self.once.call_once_force(|_| {
let value = f();
// SAFETY: We use the Once (self.once) to guarantee unique access
// to the UnsafeCell (slot).
let value: &mut T = unsafe { (&mut *slot.get()).write(value) };
// SAFETY: The value has been written to its final place in
// self.value. We do not to move it anymore, which we promise here
// with a Pin<&mut T>.
g(unsafe { Pin::new_unchecked(value) });
});
// SAFETY: The inner value has been initialized, and will not be moved
// anymore.
unsafe { Pin::new_unchecked(self.get_ref().get_unchecked()) }
}
/// Consumes the `OnceLock`, returning the wrapped value. Returns /// Consumes the `OnceLock`, returning the wrapped value. Returns
/// `None` if the cell was empty. /// `None` if the cell was empty.
/// ///

View File

@ -174,9 +174,6 @@ impl Mutex {
Mutex { inner: Spinlock::new(MutexInner::new()) } Mutex { inner: Spinlock::new(MutexInner::new()) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn lock(&self) { pub unsafe fn lock(&self) {
loop { loop {

View File

@ -31,12 +31,6 @@ impl Mutex {
Mutex { mtx: SpinIdOnceCell::new() } Mutex { mtx: SpinIdOnceCell::new() }
} }
pub unsafe fn init(&mut self) {
// Initialize `self.mtx` eagerly
let id = new_mtx().unwrap_or_else(|e| fail(e, &"acre_mtx"));
unsafe { self.mtx.set_unchecked((id, ())) };
}
/// Get the inner mutex's ID, which is lazily created. /// Get the inner mutex's ID, which is lazily created.
fn raw(&self) -> abi::ID { fn raw(&self) -> abi::ID {
match self.mtx.get_or_try_init(|| new_mtx().map(|id| (id, ()))) { match self.mtx.get_or_try_init(|| new_mtx().map(|id| (id, ()))) {

View File

@ -20,9 +20,6 @@ impl Mutex {
Mutex { inner: SpinMutex::new(WaitVariable::new(false)) } Mutex { inner: SpinMutex::new(WaitVariable::new(false)) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn lock(&self) { pub unsafe fn lock(&self) {
let mut guard = self.inner.lock(); let mut guard = self.inner.lock();

View File

@ -85,9 +85,6 @@ impl Mutex {
Mutex { futex: AtomicU32::new(UNLOCKED) } Mutex { futex: AtomicU32::new(UNLOCKED) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn try_lock(&self) -> bool { pub unsafe fn try_lock(&self) -> bool {
let thread_self = zx_thread_self(); let thread_self = zx_thread_self();

View File

@ -19,9 +19,6 @@ impl Mutex {
Self { futex: AtomicU32::new(0) } Self { futex: AtomicU32::new(0) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn try_lock(&self) -> bool { pub unsafe fn try_lock(&self) -> bool {
self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok() self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok()

View File

@ -52,7 +52,7 @@ impl Mutex {
Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) } Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
} }
#[inline] #[inline]
pub unsafe fn init(&mut self) { unsafe fn init(&mut self) {
// Issue #33770 // Issue #33770
// //
// A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have // A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have

View File

@ -16,9 +16,6 @@ impl Mutex {
Mutex { locked: Cell::new(false) } Mutex { locked: Cell::new(false) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn lock(&self) { pub unsafe fn lock(&self) {
assert_eq!(self.locked.replace(true), false, "cannot recursively acquire mutex"); assert_eq!(self.locked.replace(true), false, "cannot recursively acquire mutex");

View File

@ -37,8 +37,6 @@ impl Mutex {
pub const fn new() -> Mutex { pub const fn new() -> Mutex {
Mutex { srwlock: UnsafeCell::new(c::SRWLOCK_INIT) } Mutex { srwlock: UnsafeCell::new(c::SRWLOCK_INIT) }
} }
#[inline]
pub unsafe fn init(&mut self) {}
#[inline] #[inline]
pub unsafe fn lock(&self) { pub unsafe fn lock(&self) {

View File

@ -1,13 +1,11 @@
#[cfg(all(test, not(target_os = "emscripten")))] #[cfg(all(test, not(target_os = "emscripten")))]
mod tests; mod tests;
use super::mutex as sys;
use crate::cell::UnsafeCell; use crate::cell::UnsafeCell;
use crate::marker::PhantomPinned;
use crate::ops::Deref; use crate::ops::Deref;
use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::panic::{RefUnwindSafe, UnwindSafe};
use crate::pin::Pin;
use crate::sync::atomic::{AtomicUsize, Ordering::Relaxed}; use crate::sync::atomic::{AtomicUsize, Ordering::Relaxed};
use crate::sys::locks as sys;
/// A re-entrant mutual exclusion /// A re-entrant mutual exclusion
/// ///
@ -41,11 +39,10 @@ use crate::sys::locks as sys;
/// synchronization is left to the mutex, making relaxed memory ordering for /// synchronization is left to the mutex, making relaxed memory ordering for
/// the `owner` field fine in all cases. /// the `owner` field fine in all cases.
pub struct ReentrantMutex<T> { pub struct ReentrantMutex<T> {
mutex: sys::Mutex, mutex: sys::MovableMutex,
owner: AtomicUsize, owner: AtomicUsize,
lock_count: UnsafeCell<u32>, lock_count: UnsafeCell<u32>,
data: T, data: T,
_pinned: PhantomPinned,
} }
unsafe impl<T: Send> Send for ReentrantMutex<T> {} unsafe impl<T: Send> Send for ReentrantMutex<T> {}
@ -68,39 +65,22 @@ impl<T> RefUnwindSafe for ReentrantMutex<T> {}
/// guarded data. /// guarded data.
#[must_use = "if unused the ReentrantMutex will immediately unlock"] #[must_use = "if unused the ReentrantMutex will immediately unlock"]
pub struct ReentrantMutexGuard<'a, T: 'a> { pub struct ReentrantMutexGuard<'a, T: 'a> {
lock: Pin<&'a ReentrantMutex<T>>, lock: &'a ReentrantMutex<T>,
} }
impl<T> !Send for ReentrantMutexGuard<'_, T> {} impl<T> !Send for ReentrantMutexGuard<'_, T> {}
impl<T> ReentrantMutex<T> { impl<T> ReentrantMutex<T> {
/// Creates a new reentrant mutex in an unlocked state. /// Creates a new reentrant mutex in an unlocked state.
/// pub const fn new(t: T) -> ReentrantMutex<T> {
/// # Unsafety
///
/// This function is unsafe because it is required that `init` is called
/// once this mutex is in its final resting place, and only then are the
/// lock/unlock methods safe.
pub const unsafe fn new(t: T) -> ReentrantMutex<T> {
ReentrantMutex { ReentrantMutex {
mutex: sys::Mutex::new(), mutex: sys::MovableMutex::new(),
owner: AtomicUsize::new(0), owner: AtomicUsize::new(0),
lock_count: UnsafeCell::new(0), lock_count: UnsafeCell::new(0),
data: t, data: t,
_pinned: PhantomPinned,
} }
} }
/// Initializes this mutex so it's ready for use.
///
/// # Unsafety
///
/// Unsafe to call more than once, and must be called after this will no
/// longer move in memory.
pub unsafe fn init(self: Pin<&mut Self>) {
self.get_unchecked_mut().mutex.init()
}
/// Acquires a mutex, blocking the current thread until it is able to do so. /// Acquires a mutex, blocking the current thread until it is able to do so.
/// ///
/// This function will block the caller until it is available to acquire the mutex. /// This function will block the caller until it is available to acquire the mutex.
@ -113,15 +93,14 @@ impl<T> ReentrantMutex<T> {
/// If another user of this mutex panicked while holding the mutex, then /// If another user of this mutex panicked while holding the mutex, then
/// this call will return failure if the mutex would otherwise be /// this call will return failure if the mutex would otherwise be
/// acquired. /// acquired.
pub fn lock(self: Pin<&Self>) -> ReentrantMutexGuard<'_, T> { pub fn lock(&self) -> ReentrantMutexGuard<'_, T> {
let this_thread = current_thread_unique_ptr(); let this_thread = current_thread_unique_ptr();
// Safety: We only touch lock_count when we own the lock, // Safety: We only touch lock_count when we own the lock.
// and since self is pinned we can safely call the lock() on the mutex.
unsafe { unsafe {
if self.owner.load(Relaxed) == this_thread { if self.owner.load(Relaxed) == this_thread {
self.increment_lock_count(); self.increment_lock_count();
} else { } else {
self.mutex.lock(); self.mutex.raw_lock();
self.owner.store(this_thread, Relaxed); self.owner.store(this_thread, Relaxed);
debug_assert_eq!(*self.lock_count.get(), 0); debug_assert_eq!(*self.lock_count.get(), 0);
*self.lock_count.get() = 1; *self.lock_count.get() = 1;
@ -142,10 +121,9 @@ impl<T> ReentrantMutex<T> {
/// If another user of this mutex panicked while holding the mutex, then /// If another user of this mutex panicked while holding the mutex, then
/// this call will return failure if the mutex would otherwise be /// this call will return failure if the mutex would otherwise be
/// acquired. /// acquired.
pub fn try_lock(self: Pin<&Self>) -> Option<ReentrantMutexGuard<'_, T>> { pub fn try_lock(&self) -> Option<ReentrantMutexGuard<'_, T>> {
let this_thread = current_thread_unique_ptr(); let this_thread = current_thread_unique_ptr();
// Safety: We only touch lock_count when we own the lock, // Safety: We only touch lock_count when we own the lock.
// and since self is pinned we can safely call the try_lock on the mutex.
unsafe { unsafe {
if self.owner.load(Relaxed) == this_thread { if self.owner.load(Relaxed) == this_thread {
self.increment_lock_count(); self.increment_lock_count();
@ -179,12 +157,12 @@ impl<T> Deref for ReentrantMutexGuard<'_, T> {
impl<T> Drop for ReentrantMutexGuard<'_, T> { impl<T> Drop for ReentrantMutexGuard<'_, T> {
#[inline] #[inline]
fn drop(&mut self) { fn drop(&mut self) {
// Safety: We own the lock, and the lock is pinned. // Safety: We own the lock.
unsafe { unsafe {
*self.lock.lock_count.get() -= 1; *self.lock.lock_count.get() -= 1;
if *self.lock.lock_count.get() == 0 { if *self.lock.lock_count.get() == 0 {
self.lock.owner.store(0, Relaxed); self.lock.owner.store(0, Relaxed);
self.lock.mutex.unlock(); self.lock.mutex.raw_unlock();
} }
} }
} }

View File

@ -1,18 +1,11 @@
use crate::boxed::Box;
use crate::cell::RefCell; use crate::cell::RefCell;
use crate::pin::Pin;
use crate::sync::Arc; use crate::sync::Arc;
use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
use crate::thread; use crate::thread;
#[test] #[test]
fn smoke() { fn smoke() {
let m = unsafe { let m = ReentrantMutex::new(());
let mut m = Box::pin(ReentrantMutex::new(()));
m.as_mut().init();
m
};
let m = m.as_ref();
{ {
let a = m.lock(); let a = m.lock();
{ {
@ -29,20 +22,15 @@ fn smoke() {
#[test] #[test]
fn is_mutex() { fn is_mutex() {
let m = unsafe { let m = Arc::new(ReentrantMutex::new(RefCell::new(0)));
// FIXME: Simplify this if Arc gets an Arc::get_pin_mut.
let mut m = Arc::new(ReentrantMutex::new(RefCell::new(0)));
Pin::new_unchecked(Arc::get_mut_unchecked(&mut m)).init();
Pin::new_unchecked(m)
};
let m2 = m.clone(); let m2 = m.clone();
let lock = m.as_ref().lock(); let lock = m.lock();
let child = thread::spawn(move || { let child = thread::spawn(move || {
let lock = m2.as_ref().lock(); let lock = m2.lock();
assert_eq!(*lock.borrow(), 4950); assert_eq!(*lock.borrow(), 4950);
}); });
for i in 0..100 { for i in 0..100 {
let lock = m.as_ref().lock(); let lock = m.lock();
*lock.borrow_mut() += i; *lock.borrow_mut() += i;
} }
drop(lock); drop(lock);
@ -51,22 +39,17 @@ fn is_mutex() {
#[test] #[test]
fn trylock_works() { fn trylock_works() {
let m = unsafe { let m = Arc::new(ReentrantMutex::new(()));
// FIXME: Simplify this if Arc gets an Arc::get_pin_mut.
let mut m = Arc::new(ReentrantMutex::new(()));
Pin::new_unchecked(Arc::get_mut_unchecked(&mut m)).init();
Pin::new_unchecked(m)
};
let m2 = m.clone(); let m2 = m.clone();
let _lock = m.as_ref().try_lock(); let _lock = m.try_lock();
let _lock2 = m.as_ref().try_lock(); let _lock2 = m.try_lock();
thread::spawn(move || { thread::spawn(move || {
let lock = m2.as_ref().try_lock(); let lock = m2.try_lock();
assert!(lock.is_none()); assert!(lock.is_none());
}) })
.join() .join()
.unwrap(); .unwrap();
let _lock3 = m.as_ref().try_lock(); let _lock3 = m.try_lock();
} }
pub struct Answer<'a>(pub ReentrantMutexGuard<'a, RefCell<u32>>); pub struct Answer<'a>(pub ReentrantMutexGuard<'a, RefCell<u32>>);