Skip to content

Commit

Permalink
Rollup merge of rust-lang#100576 - joboet:movable_const_remutex, r=Ma…
Browse files Browse the repository at this point in the history
…rk-Simulacrum

Make `ReentrantMutex` movable and `const`

As `MovableMutex` is now `const`, it can be used to simplify the implementation and interface of the internal reentrant mutex type. Consequently, the standard output and error streams do not need to be wrapped in `OnceLock` and `OnceLock::get_or_init_pin()` can be removed. Moving the initialization of the `Stdout` buffer to `StdoutLock` means that there is only one synchronization primitive required, at the cost of probably two instructions for every read/write.
  • Loading branch information
matthiaskrgr authored Aug 28, 2022
2 parents 4d9d2b2 + 9d222e9 commit 08bf04a
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 173 deletions.
73 changes: 37 additions & 36 deletions library/std/src/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ mod tests;

use crate::io::prelude::*;

use crate::cell::{Cell, RefCell};
use crate::cell::{Cell, RefCell, RefMut};
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<Option<LineWriter<StdoutRaw>>>>,
}

/// A locked reference to the [`Stdout`] handle.
Expand All @@ -548,10 +547,11 @@ pub struct Stdout {
#[must_use = "if unused stdout will immediately unlock"]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct StdoutLock<'a> {
inner: ReentrantMutexGuard<'a, RefCell<LineWriter<StdoutRaw>>>,
inner: ReentrantMutexGuard<'a, RefCell<Option<LineWriter<StdoutRaw>>>>,
}

static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLock::new();
static STDOUT: ReentrantMutex<RefCell<Option<LineWriter<StdoutRaw>>>> =
ReentrantMutex::new(RefCell::new(None));

/// Constructs a new handle to the standard output of the current process.
///
Expand Down Expand Up @@ -602,25 +602,18 @@ static STDOUT: OnceLock<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = OnceLo
#[must_use]
#[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() },
),
}
Stdout { inner: &STDOUT }
}

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.try_lock() {
*lock.borrow_mut() = Some(LineWriter::with_capacity(0, stdout_raw()));
}
}

Expand Down Expand Up @@ -712,26 +705,38 @@ impl Write for &Stdout {
}
}

impl StdoutLock<'_> {
#[inline]
fn inner(&self) -> RefMut<'_, LineWriter<StdoutRaw>> {
#[cold]
fn init() -> LineWriter<StdoutRaw> {
LineWriter::new(stdout_raw())
}

RefMut::map(self.inner.borrow_mut(), |w| w.get_or_insert_with(init))
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Write for StdoutLock<'_> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
self.inner.borrow_mut().write(buf)
self.inner().write(buf)
}
fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result<usize> {
self.inner.borrow_mut().write_vectored(bufs)
self.inner().write_vectored(bufs)
}
#[inline]
fn is_write_vectored(&self) -> bool {
self.inner.borrow_mut().is_write_vectored()
self.inner().is_write_vectored()
}
fn flush(&mut self) -> io::Result<()> {
self.inner.borrow_mut().flush()
self.inner().flush()
}
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
self.inner.borrow_mut().write_all(buf)
self.inner().write_all(buf)
}
fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> {
self.inner.borrow_mut().write_all_vectored(bufs)
self.inner().write_all_vectored(bufs)
}
}

Expand Down Expand Up @@ -761,7 +766,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 +839,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/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
Loading

0 comments on commit 08bf04a

Please sign in to comment.