Skip to content

Commit

Permalink
Fix abort-on-eprintln during process shutdown
Browse files Browse the repository at this point in the history
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
alexcrichton committed Mar 20, 2020
1 parent 23de827 commit 5edaa7e
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
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
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
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
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
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
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
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
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
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

0 comments on commit 5edaa7e

Please sign in to comment.