Skip to content

Commit cb8af5e

Browse files
authored
Rollup merge of rust-lang#69955 - alexcrichton:stderr-infallible, r=sfackler
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.
2 parents b09b114 + 2c22da0 commit cb8af5e

File tree

11 files changed

+145
-100
lines changed

11 files changed

+145
-100
lines changed

src/libstd/io/stdio.rs

+31-18
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::cell::RefCell;
66
use crate::fmt;
77
use crate::io::lazy::Lazy;
88
use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter};
9-
use crate::sync::{Arc, Mutex, MutexGuard};
9+
use crate::sync::{Arc, Mutex, MutexGuard, Once};
1010
use crate::sys::stdio;
1111
use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
1212
use crate::thread::LocalKey;
@@ -493,7 +493,11 @@ pub fn stdout() -> Stdout {
493493
Ok(stdout) => Maybe::Real(stdout),
494494
_ => Maybe::Fake,
495495
};
496-
Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))))
496+
unsafe {
497+
let ret = Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))));
498+
ret.init();
499+
return ret;
500+
}
497501
}
498502
}
499503

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

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

587591
/// A locked reference to the `Stderr` handle.
@@ -639,19 +643,28 @@ pub struct StderrLock<'a> {
639643
/// ```
640644
#[stable(feature = "rust1", since = "1.0.0")]
641645
pub fn stderr() -> Stderr {
642-
static INSTANCE: Lazy<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> = Lazy::new();
643-
return Stderr {
644-
inner: unsafe { INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown") },
645-
};
646-
647-
fn stderr_init() -> Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> {
648-
// This must not reentrantly access `INSTANCE`
649-
let stderr = match stderr_raw() {
650-
Ok(stderr) => Maybe::Real(stderr),
651-
_ => Maybe::Fake,
652-
};
653-
Arc::new(ReentrantMutex::new(RefCell::new(stderr)))
654-
}
646+
// Note that unlike `stdout()` we don't use `Lazy` here which registers a
647+
// destructor. Stderr is not buffered nor does the `stderr_raw` type consume
648+
// any owned resources, so there's no need to run any destructors at some
649+
// point in the future.
650+
//
651+
// This has the added benefit of allowing `stderr` to be usable during
652+
// process shutdown as well!
653+
static INSTANCE: ReentrantMutex<RefCell<Maybe<StderrRaw>>> =
654+
unsafe { ReentrantMutex::new(RefCell::new(Maybe::Fake)) };
655+
656+
// When accessing stderr we need one-time initialization of the reentrant
657+
// mutex, followed by one-time detection of whether we actually have a
658+
// stderr handle or not. Afterwards we can just always use the now-filled-in
659+
// `INSTANCE` value.
660+
static INIT: Once = Once::new();
661+
INIT.call_once(|| unsafe {
662+
INSTANCE.init();
663+
if let Ok(stderr) = stderr_raw() {
664+
*INSTANCE.lock().borrow_mut() = Maybe::Real(stderr);
665+
}
666+
});
667+
return Stderr { inner: &INSTANCE };
655668
}
656669

657670
impl Stderr {
@@ -677,7 +690,7 @@ impl Stderr {
677690
/// ```
678691
#[stable(feature = "rust1", since = "1.0.0")]
679692
pub fn lock(&self) -> StderrLock<'_> {
680-
StderrLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) }
693+
StderrLock { inner: self.inner.lock() }
681694
}
682695
}
683696

src/libstd/sys/cloudabi/mutex.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,16 @@ pub struct ReentrantMutex {
5353
}
5454

5555
impl ReentrantMutex {
56-
pub unsafe fn uninitialized() -> ReentrantMutex {
56+
pub const unsafe fn uninitialized() -> ReentrantMutex {
5757
ReentrantMutex {
5858
lock: UnsafeCell::new(MaybeUninit::uninit()),
5959
recursion: UnsafeCell::new(MaybeUninit::uninit()),
6060
}
6161
}
6262

63-
pub unsafe fn init(&mut self) {
64-
self.lock = UnsafeCell::new(MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0)));
65-
self.recursion = UnsafeCell::new(MaybeUninit::new(0));
63+
pub unsafe fn init(&self) {
64+
*self.lock.get() = MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0));
65+
*self.recursion.get() = MaybeUninit::new(0);
6666
}
6767

6868
pub unsafe fn try_lock(&self) -> bool {

src/libstd/sys/hermit/mutex.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ pub struct ReentrantMutex {
4646
}
4747

4848
impl ReentrantMutex {
49-
pub unsafe fn uninitialized() -> ReentrantMutex {
49+
pub const unsafe fn uninitialized() -> ReentrantMutex {
5050
ReentrantMutex { inner: ptr::null() }
5151
}
5252

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

5858
#[inline]

src/libstd/sys/sgx/mutex.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl ReentrantMutex {
7575
}
7676

7777
#[inline]
78-
pub unsafe fn init(&mut self) {}
78+
pub unsafe fn init(&self) {}
7979

8080
#[inline]
8181
pub unsafe fn lock(&self) {

src/libstd/sys/unix/mutex.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {}
9292
unsafe impl Sync for ReentrantMutex {}
9393

9494
impl ReentrantMutex {
95-
pub unsafe fn uninitialized() -> ReentrantMutex {
95+
pub const unsafe fn uninitialized() -> ReentrantMutex {
9696
ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
9797
}
9898

99-
pub unsafe fn init(&mut self) {
99+
pub unsafe fn init(&self) {
100100
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
101101
let result = libc::pthread_mutexattr_init(attr.as_mut_ptr());
102102
debug_assert_eq!(result, 0);

src/libstd/sys/vxworks/mutex.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {}
9292
unsafe impl Sync for ReentrantMutex {}
9393

9494
impl ReentrantMutex {
95-
pub unsafe fn uninitialized() -> ReentrantMutex {
95+
pub const unsafe fn uninitialized() -> ReentrantMutex {
9696
ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
9797
}
9898

99-
pub unsafe fn init(&mut self) {
99+
pub unsafe fn init(&self) {
100100
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
101101
let result = libc::pthread_mutexattr_init(attr.as_mut_ptr());
102102
debug_assert_eq!(result, 0);

src/libstd/sys/wasm/mutex.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ impl Mutex {
4747
pub struct ReentrantMutex {}
4848

4949
impl ReentrantMutex {
50-
pub unsafe fn uninitialized() -> ReentrantMutex {
50+
pub const unsafe fn uninitialized() -> ReentrantMutex {
5151
ReentrantMutex {}
5252
}
5353

54-
pub unsafe fn init(&mut self) {}
54+
pub unsafe fn init(&self) {}
5555

5656
pub unsafe fn lock(&self) {}
5757

src/libstd/sys/wasm/mutex_atomics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ unsafe impl Sync for ReentrantMutex {}
8080
// released when this recursion counter reaches 0.
8181

8282
impl ReentrantMutex {
83-
pub unsafe fn uninitialized() -> ReentrantMutex {
83+
pub const unsafe fn uninitialized() -> ReentrantMutex {
8484
ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) }
8585
}
8686

87-
pub unsafe fn init(&mut self) {
87+
pub unsafe fn init(&self) {
8888
// nothing to do...
8989
}
9090

src/libstd/sys/windows/mutex.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ impl Mutex {
109109
0 => {}
110110
n => return n as *mut _,
111111
}
112-
let mut re = box ReentrantMutex::uninitialized();
112+
let re = box ReentrantMutex::uninitialized();
113113
re.init();
114114
let re = Box::into_raw(re);
115115
match self.lock.compare_and_swap(0, re as usize, Ordering::SeqCst) {
@@ -157,11 +157,11 @@ unsafe impl Send for ReentrantMutex {}
157157
unsafe impl Sync for ReentrantMutex {}
158158

159159
impl ReentrantMutex {
160-
pub fn uninitialized() -> ReentrantMutex {
160+
pub const fn uninitialized() -> ReentrantMutex {
161161
ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninit()) }
162162
}
163163

164-
pub unsafe fn init(&mut self) {
164+
pub unsafe fn init(&self) {
165165
c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr());
166166
}
167167

0 commit comments

Comments
 (0)