From 93ba97f97b487fb4c8da4842078d7595ecc2a4a5 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 10 Oct 2020 20:22:15 +0200 Subject: [PATCH] Pin the ReentrantMutexes in sys::io::stdio. The code before this change misused the ReentrantMutexes, by calling init() on them and moving them afterwards. Now that ReentrantMutex requires Pin for init(), this mistake is no longer easy to make. --- library/std/src/io/stdio.rs | 54 ++++++++++++++++++++----------------- library/std/src/lib.rs | 1 + 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 9b6bd70523da..b68e538c19fb 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -9,6 +9,7 @@ use crate::cell::RefCell; use crate::fmt; use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter}; use crate::lazy::SyncOnceCell; +use crate::pin::Pin; use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sync::{Mutex, MutexGuard}; use crate::sys::stdio; @@ -488,7 +489,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: &'static ReentrantMutex>>, + inner: Pin<&'static ReentrantMutex>>>, } /// A locked reference to the `Stdout` handle. @@ -548,25 +549,29 @@ pub struct StdoutLock<'a> { pub fn stdout() -> Stdout { static INSTANCE: SyncOnceCell>>> = SyncOnceCell::new(); + + fn cleanup() { + if let Some(instance) = INSTANCE.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()); + } + } + } + Stdout { - inner: INSTANCE.get_or_init(|| unsafe { - let _ = sys_common::at_exit(|| { - if let Some(instance) = INSTANCE.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) = instance.try_lock() { - *lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw()); - } - } - }); - let r = ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))); - r.init(); - r - }), + inner: Pin::static_ref(&INSTANCE).get_or_init_pin( + || unsafe { + let _ = sys_common::at_exit(cleanup); + ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))) + }, + |mutex| unsafe { mutex.init() }, + ), } } @@ -698,7 +703,7 @@ impl fmt::Debug for StdoutLock<'_> { /// an error. #[stable(feature = "rust1", since = "1.0.0")] pub struct Stderr { - inner: &'static ReentrantMutex>, + inner: Pin<&'static ReentrantMutex>>, } /// A locked reference to the `Stderr` handle. @@ -760,11 +765,10 @@ pub fn stderr() -> Stderr { static INSTANCE: SyncOnceCell>> = SyncOnceCell::new(); Stderr { - inner: INSTANCE.get_or_init(|| unsafe { - let r = ReentrantMutex::new(RefCell::new(stderr_raw())); - r.init(); - r - }), + inner: Pin::static_ref(&INSTANCE).get_or_init_pin( + || unsafe { ReentrantMutex::new(RefCell::new(stderr_raw())) }, + |mutex| unsafe { mutex.init() }, + ), } } diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index e9ac7875cc26..5aa9d6a3562f 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -291,6 +291,7 @@ #![feature(panic_info_message)] #![feature(panic_internals)] #![feature(panic_unwind)] +#![feature(pin_static_ref)] #![feature(prelude_import)] #![feature(ptr_internals)] #![feature(raw)]