Skip to content

Commit

Permalink
Rollup merge of #69955 - alexcrichton:stderr-infallible, r=sfackler
Browse files Browse the repository at this point in the history
Fix abort-on-eprintln during process shutdown

This commit fixes an issue where if `eprintln!` is used in a TLS
destructor it can accidentally cause the process to abort. TLS
destructors are executed after `main` returns on the main thread, and at
this point we've also deinitialized global `Lazy` values like those
which store the `Stderr` and `Stdout` internals. This means that despite
handling TLS not being accessible in `eprintln!`, we will fail due to
not being able to call `stderr()`. This means that we'll double-panic
quickly because panicking also attempt to write to stderr.

The fix here is to reimplement the global stderr handle to avoid the
need for destruction. This avoids the need for `Lazy` as well as the
hidden panic inside of the `stderr` function.

Overall this should improve the robustness of printing errors and/or
panics in weird situations, since the `stderr` accessor should be
infallible in more situations.
  • Loading branch information
Dylan-DPC authored Mar 21, 2020
2 parents 8deeac1 + 5edaa7e commit 276b54e
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 100 deletions.
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) {
*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

0 comments on commit 276b54e

Please sign in to comment.