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

Make ReentrantMutex movable and const #100576

Merged
merged 1 commit into from
Sep 5, 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
41 changes: 16 additions & 25 deletions library/std/src/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::io::prelude::*;
use crate::cell::{Cell, RefCell};
use crate::fmt;
use crate::io::{self, BufReader, IoSlice, IoSliceMut, LineWriter, Lines};
use crate::pin::Pin;
use crate::sync::atomic::{AtomicBool, Ordering};
use crate::sync::{Arc, Mutex, MutexGuard, OnceLock};
use crate::sys::stdio;
Expand Down Expand Up @@ -526,7 +525,7 @@ pub struct Stdout {
// 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
// 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.
Expand Down Expand Up @@ -603,24 +602,20 @@ static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLo
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stdout() -> Stdout {
Stdout {
inner: Pin::static_ref(&STDOUT).get_or_init_pin(
|| unsafe { ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))) },
|mutex| unsafe { mutex.init() },
),
inner: STDOUT
.get_or_init(|| ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw())))),
}
}

pub fn cleanup() {
if let Some(instance) = STDOUT.get() {
// Flush the data and disable buffering during shutdown
// by replacing the line writer by one with zero
// buffering capacity.
// We use try_lock() instead of lock(), because someone
// might have leaked a StdoutLock, which would
// otherwise cause a deadlock here.
if let Some(lock) = Pin::static_ref(instance).try_lock() {
*lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
}
// Flush the data and disable buffering during shutdown
// by replacing the line writer by one with zero
// buffering capacity.
// We use try_lock() instead of lock(), because someone
// might have leaked a StdoutLock, which would
// otherwise cause a deadlock here.
if let Some(lock) = STDOUT.get().and_then(ReentrantMutex::try_lock) {
*lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
}
}

Expand Down Expand Up @@ -761,7 +756,7 @@ impl fmt::Debug for StdoutLock<'_> {
/// standard library or via raw Windows API calls, will fail.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stderr {
inner: Pin<&'static ReentrantMutex<RefCell<StderrRaw>>>,
inner: &'static ReentrantMutex<RefCell<StderrRaw>>,
}

/// A locked reference to the [`Stderr`] handle.
Expand Down Expand Up @@ -834,16 +829,12 @@ pub struct StderrLock<'a> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stderr() -> Stderr {
// 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
static INSTANCE: OnceLock<ReentrantMutex<RefCell<StderrRaw>>> = OnceLock::new();
static INSTANCE: ReentrantMutex<RefCell<StderrRaw>> =
ReentrantMutex::new(RefCell::new(stderr_raw()));

Stderr {
inner: Pin::static_ref(&INSTANCE).get_or_init_pin(
|| unsafe { ReentrantMutex::new(RefCell::new(stderr_raw())) },
|mutex| unsafe { mutex.init() },
),
}
Stderr { inner: &INSTANCE }
}

impl Stderr {
Expand Down
55 changes: 0 additions & 55 deletions library/std/src/sync/once_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::fmt;
use crate::marker::PhantomData;
use crate::mem::MaybeUninit;
use crate::panic::{RefUnwindSafe, UnwindSafe};
use crate::pin::Pin;
use crate::sync::Once;

/// A synchronization primitive which can be written to only once.
Expand Down Expand Up @@ -223,60 +222,6 @@ impl<T> OnceLock<T> {
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
/// `None` if the cell was empty.
///
Expand Down
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 @@ -174,9 +174,6 @@ impl Mutex {
Mutex { inner: Spinlock::new(MutexInner::new()) }
}

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

#[inline]
pub unsafe fn lock(&self) {
loop {
Expand Down
6 changes: 0 additions & 6 deletions library/std/src/sys/itron/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ impl Mutex {
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.
fn raw(&self) -> abi::ID {
match self.mtx.get_or_try_init(|| new_mtx().map(|id| (id, ()))) {
Expand Down
3 changes: 0 additions & 3 deletions library/std/src/sys/sgx/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ impl Mutex {
Mutex { inner: SpinMutex::new(WaitVariable::new(false)) }
}

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

#[inline]
pub unsafe fn lock(&self) {
let mut guard = self.inner.lock();
Expand Down
3 changes: 0 additions & 3 deletions library/std/src/sys/unix/locks/fuchsia_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ impl Mutex {
Mutex { futex: AtomicU32::new(UNLOCKED) }
}

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

#[inline]
pub unsafe fn try_lock(&self) -> bool {
let thread_self = zx_thread_self();
Expand Down
3 changes: 0 additions & 3 deletions library/std/src/sys/unix/locks/futex_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ impl Mutex {
Self { futex: AtomicU32::new(0) }
}

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

#[inline]
pub unsafe fn try_lock(&self) -> bool {
self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok()
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unix/locks/pthread_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl Mutex {
Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
}
#[inline]
pub unsafe fn init(&mut self) {
unsafe fn init(&mut self) {
// Issue #33770
//
// A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have
Expand Down
3 changes: 0 additions & 3 deletions library/std/src/sys/unsupported/locks/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ impl Mutex {
Mutex { locked: Cell::new(false) }
}

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

#[inline]
pub unsafe fn lock(&self) {
assert_eq!(self.locked.replace(true), false, "cannot recursively acquire mutex");
Expand Down
2 changes: 0 additions & 2 deletions library/std/src/sys/windows/locks/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ impl Mutex {
pub const fn new() -> Mutex {
Mutex { srwlock: UnsafeCell::new(c::SRWLOCK_INIT) }
}
#[inline]
pub unsafe fn init(&mut self) {}

#[inline]
pub unsafe fn lock(&self) {
Expand Down
46 changes: 12 additions & 34 deletions library/std/src/sys_common/remutex.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
#[cfg(all(test, not(target_os = "emscripten")))]
mod tests;

use super::mutex as sys;
use crate::cell::UnsafeCell;
use crate::marker::PhantomPinned;
use crate::ops::Deref;
use crate::panic::{RefUnwindSafe, UnwindSafe};
use crate::pin::Pin;
use crate::sync::atomic::{AtomicUsize, Ordering::Relaxed};
use crate::sys::locks as sys;

/// A re-entrant mutual exclusion
///
Expand Down Expand Up @@ -41,11 +39,10 @@ use crate::sys::locks as sys;
/// synchronization is left to the mutex, making relaxed memory ordering for
/// the `owner` field fine in all cases.
pub struct ReentrantMutex<T> {
mutex: sys::Mutex,
mutex: sys::MovableMutex,
owner: AtomicUsize,
lock_count: UnsafeCell<u32>,
data: T,
_pinned: PhantomPinned,
}

unsafe impl<T: Send> Send for ReentrantMutex<T> {}
Expand All @@ -68,39 +65,22 @@ impl<T> RefUnwindSafe for ReentrantMutex<T> {}
/// guarded data.
#[must_use = "if unused the ReentrantMutex will immediately unlock"]
pub struct ReentrantMutexGuard<'a, T: 'a> {
lock: Pin<&'a ReentrantMutex<T>>,
lock: &'a ReentrantMutex<T>,
}

impl<T> !Send for ReentrantMutexGuard<'_, T> {}

impl<T> ReentrantMutex<T> {
/// Creates a new reentrant mutex in an unlocked state.
///
/// # 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> {
pub const fn new(t: T) -> ReentrantMutex<T> {
ReentrantMutex {
mutex: sys::Mutex::new(),
mutex: sys::MovableMutex::new(),
owner: AtomicUsize::new(0),
lock_count: UnsafeCell::new(0),
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.
///
/// This function will block the caller until it is available to acquire the mutex.
Expand All @@ -113,15 +93,14 @@ impl<T> ReentrantMutex<T> {
/// If another user of this mutex panicked while holding the mutex, then
/// this call will return failure if the mutex would otherwise be
/// acquired.
pub fn lock(self: Pin<&Self>) -> ReentrantMutexGuard<'_, T> {
pub fn lock(&self) -> ReentrantMutexGuard<'_, T> {
let this_thread = current_thread_unique_ptr();
// 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.
// Safety: We only touch lock_count when we own the lock.
unsafe {
if self.owner.load(Relaxed) == this_thread {
self.increment_lock_count();
} else {
self.mutex.lock();
self.mutex.raw_lock();
self.owner.store(this_thread, Relaxed);
debug_assert_eq!(*self.lock_count.get(), 0);
*self.lock_count.get() = 1;
Expand All @@ -142,10 +121,9 @@ impl<T> ReentrantMutex<T> {
/// If another user of this mutex panicked while holding the mutex, then
/// this call will return failure if the mutex would otherwise be
/// 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();
// 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.
// Safety: We only touch lock_count when we own the lock.
unsafe {
if self.owner.load(Relaxed) == this_thread {
self.increment_lock_count();
Expand Down Expand Up @@ -179,12 +157,12 @@ impl<T> Deref for ReentrantMutexGuard<'_, T> {
impl<T> Drop for ReentrantMutexGuard<'_, T> {
#[inline]
fn drop(&mut self) {
// Safety: We own the lock, and the lock is pinned.
// Safety: We own the lock.
unsafe {
*self.lock.lock_count.get() -= 1;
if *self.lock.lock_count.get() == 0 {
self.lock.owner.store(0, Relaxed);
self.lock.mutex.unlock();
self.lock.mutex.raw_unlock();
}
}
}
Expand Down
Loading