Skip to content

Commit 39647ab

Browse files
committed
fix interleaved panic output
previously, we only held a lock for printing the backtrace itself. since all threads were printing to the same file descriptor, that meant random output in the default panic hook would be interleaved with the backtrace. now, we hold the lock for the full duration of the hook, and the output is ordered.
1 parent 625bcc4 commit 39647ab

File tree

2 files changed

+31
-30
lines changed

2 files changed

+31
-30
lines changed

std/src/panicking.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,20 @@ fn default_hook(info: &PanicHookInfo<'_>) {
253253
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");
254254

255255
let write = |err: &mut dyn crate::io::Write| {
256+
// Use a lock to prevent mixed output in multithreading context.
257+
// Some platforms also require it when printing a backtrace, like `SymFromAddr` on Windows.
258+
let mut lock = backtrace::lock();
256259
let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}");
257260

258261
static FIRST_PANIC: AtomicBool = AtomicBool::new(true);
259262

260263
match backtrace {
264+
// SAFETY: we took out a lock just a second ago.
261265
Some(BacktraceStyle::Short) => {
262-
drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Short))
266+
drop(lock.print(err, crate::backtrace_rs::PrintFmt::Short))
263267
}
264268
Some(BacktraceStyle::Full) => {
265-
drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Full))
269+
drop(lock.print(err, crate::backtrace_rs::PrintFmt::Full))
266270
}
267271
Some(BacktraceStyle::Off) => {
268272
if FIRST_PANIC.swap(false, Ordering::Relaxed) {

std/src/sys/backtrace.rs

+25-28
Original file line numberDiff line numberDiff line change
@@ -7,44 +7,41 @@ use crate::fmt;
77
use crate::io;
88
use crate::io::prelude::*;
99
use crate::path::{self, Path, PathBuf};
10-
use crate::sync::{Mutex, PoisonError};
10+
use crate::sync::{Mutex, MutexGuard, PoisonError};
1111

1212
/// Max number of frames to print.
1313
const MAX_NB_FRAMES: usize = 100;
1414

15-
pub fn lock() -> impl Drop {
15+
pub(crate) struct BacktraceLock<'a>(#[allow(dead_code)] MutexGuard<'a, ()>);
16+
17+
pub(crate) fn lock<'a>() -> BacktraceLock<'a> {
1618
static LOCK: Mutex<()> = Mutex::new(());
17-
LOCK.lock().unwrap_or_else(PoisonError::into_inner)
19+
BacktraceLock(LOCK.lock().unwrap_or_else(PoisonError::into_inner))
1820
}
1921

20-
/// Prints the current backtrace.
21-
pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
22-
// There are issues currently linking libbacktrace into tests, and in
23-
// general during std's own unit tests we're not testing this path. In
24-
// test mode immediately return here to optimize away any references to the
25-
// libbacktrace symbols
26-
if cfg!(test) {
27-
return Ok(());
28-
}
29-
30-
// Use a lock to prevent mixed output in multithreading context.
31-
// Some platforms also requires it, like `SymFromAddr` on Windows.
32-
unsafe {
33-
let _lock = lock();
34-
_print(w, format)
35-
}
36-
}
22+
impl BacktraceLock<'_> {
23+
/// Prints the current backtrace.
24+
///
25+
/// NOTE: this function is not Sync. The caller must hold a mutex lock, or there must be only one thread in the program.
26+
pub(crate) fn print(&mut self, w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
27+
// There are issues currently linking libbacktrace into tests, and in
28+
// general during std's own unit tests we're not testing this path. In
29+
// test mode immediately return here to optimize away any references to the
30+
// libbacktrace symbols
31+
if cfg!(test) {
32+
return Ok(());
33+
}
3734

38-
unsafe fn _print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
39-
struct DisplayBacktrace {
40-
format: PrintFmt,
41-
}
42-
impl fmt::Display for DisplayBacktrace {
43-
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
44-
unsafe { _print_fmt(fmt, self.format) }
35+
struct DisplayBacktrace {
36+
format: PrintFmt,
37+
}
38+
impl fmt::Display for DisplayBacktrace {
39+
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
40+
unsafe { _print_fmt(fmt, self.format) }
41+
}
4542
}
43+
write!(w, "{}", DisplayBacktrace { format })
4644
}
47-
write!(w, "{}", DisplayBacktrace { format })
4845
}
4946

5047
unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::Result {

0 commit comments

Comments
 (0)