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

Fix abort-on-eprintln during process shutdown #69955

Merged
merged 1 commit into from
Mar 21, 2020
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
49 changes: 31 additions & 18 deletions src/libstd/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::cell::RefCell;
use crate::fmt;
use crate::io::lazy::Lazy;
use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter};
use crate::sync::{Arc, Mutex, MutexGuard};
use crate::sync::{Arc, Mutex, MutexGuard, Once};
use crate::sys::stdio;
use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
use crate::thread::LocalKey;
Expand Down Expand Up @@ -493,7 +493,11 @@ pub fn stdout() -> Stdout {
Ok(stdout) => Maybe::Real(stdout),
_ => Maybe::Fake,
};
Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))))
unsafe {
let ret = Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))));
ret.init();
return ret;
}
}
}

Expand All @@ -520,7 +524,7 @@ impl Stdout {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn lock(&self) -> StdoutLock<'_> {
StdoutLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
StdoutLock { inner: self.inner.lock() }
}
}

Expand Down Expand Up @@ -581,7 +585,7 @@ impl fmt::Debug for StdoutLock<'_> {
/// an error.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stderr {
inner: Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>>,
inner: &'static ReentrantMutex<RefCell<Maybe<StderrRaw>>>,
}

/// A locked reference to the `Stderr` handle.
Expand Down Expand Up @@ -639,19 +643,28 @@ pub struct StderrLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stderr() -> Stderr {
static INSTANCE: Lazy<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> = Lazy::new();
return Stderr {
inner: unsafe { INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown") },
};

fn stderr_init() -> Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> {
// This must not reentrantly access `INSTANCE`
let stderr = match stderr_raw() {
Ok(stderr) => Maybe::Real(stderr),
_ => Maybe::Fake,
};
Arc::new(ReentrantMutex::new(RefCell::new(stderr)))
}
// Note that unlike `stdout()` we don't use `Lazy` here which registers a
// destructor. Stderr is not buffered nor does the `stderr_raw` type consume
// any owned resources, so there's no need to run any destructors at some
// point in the future.
//
// This has the added benefit of allowing `stderr` to be usable during
// process shutdown as well!
static INSTANCE: ReentrantMutex<RefCell<Maybe<StderrRaw>>> =
unsafe { ReentrantMutex::new(RefCell::new(Maybe::Fake)) };

// When accessing stderr we need one-time initialization of the reentrant
// mutex, followed by one-time detection of whether we actually have a
// stderr handle or not. Afterwards we can just always use the now-filled-in
// `INSTANCE` value.
static INIT: Once = Once::new();
INIT.call_once(|| unsafe {
INSTANCE.init();
if let Ok(stderr) = stderr_raw() {
*INSTANCE.lock().borrow_mut() = Maybe::Real(stderr);
}
});
return Stderr { inner: &INSTANCE };
}

impl Stderr {
Expand All @@ -677,7 +690,7 @@ impl Stderr {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn lock(&self) -> StderrLock<'_> {
StderrLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
StderrLock { inner: self.inner.lock() }
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/libstd/sys/cloudabi/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ pub struct ReentrantMutex {
}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex {
lock: UnsafeCell::new(MaybeUninit::uninit()),
recursion: UnsafeCell::new(MaybeUninit::uninit()),
}
}

pub unsafe fn init(&mut self) {
self.lock = UnsafeCell::new(MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0)));
self.recursion = UnsafeCell::new(MaybeUninit::new(0));
pub unsafe fn init(&self) {
Copy link
Member

@sfackler sfackler Mar 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily related to this PR, but why is this split out to a separate method in the first place? It seems like the lock's constructor would still be able to be const without needing to go through an uninitialized stage, right?

EDIT: Nevermind, it's platform specific!

*self.lock.get() = MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0));
*self.recursion.get() = MaybeUninit::new(0);
}

pub unsafe fn try_lock(&self) -> bool {
Expand Down
6 changes: 3 additions & 3 deletions src/libstd/sys/hermit/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ pub struct ReentrantMutex {
}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: ptr::null() }
}

#[inline]
pub unsafe fn init(&mut self) {
let _ = abi::recmutex_init(&mut self.inner as *mut *const c_void);
pub unsafe fn init(&self) {
let _ = abi::recmutex_init(&self.inner as *const *const c_void as *mut _);
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/sgx/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl ReentrantMutex {
}

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

#[inline]
pub unsafe fn lock(&self) {
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/unix/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {}
unsafe impl Sync for ReentrantMutex {}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
}

pub unsafe fn init(&mut self) {
pub unsafe fn init(&self) {
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
let result = libc::pthread_mutexattr_init(attr.as_mut_ptr());
debug_assert_eq!(result, 0);
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/vxworks/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {}
unsafe impl Sync for ReentrantMutex {}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
}

pub unsafe fn init(&mut self) {
pub unsafe fn init(&self) {
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
let result = libc::pthread_mutexattr_init(attr.as_mut_ptr());
debug_assert_eq!(result, 0);
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/wasm/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ impl Mutex {
pub struct ReentrantMutex {}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex {}
}

pub unsafe fn init(&mut self) {}
pub unsafe fn init(&self) {}

pub unsafe fn lock(&self) {}

Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/wasm/mutex_atomics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ unsafe impl Sync for ReentrantMutex {}
// released when this recursion counter reaches 0.

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
pub const unsafe fn uninitialized() -> ReentrantMutex {
ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) }
}

pub unsafe fn init(&mut self) {
pub unsafe fn init(&self) {
// nothing to do...
}

Expand Down
6 changes: 3 additions & 3 deletions src/libstd/sys/windows/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Mutex {
0 => {}
n => return n as *mut _,
}
let mut re = box ReentrantMutex::uninitialized();
let re = box ReentrantMutex::uninitialized();
re.init();
let re = Box::into_raw(re);
match self.lock.compare_and_swap(0, re as usize, Ordering::SeqCst) {
Expand Down Expand Up @@ -157,11 +157,11 @@ unsafe impl Send for ReentrantMutex {}
unsafe impl Sync for ReentrantMutex {}

impl ReentrantMutex {
pub fn uninitialized() -> ReentrantMutex {
pub const fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninit()) }
}

pub unsafe fn init(&mut self) {
pub unsafe fn init(&self) {
c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}

Expand Down
Loading