Skip to content

Commit 5ce6bb0

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 de14f1f commit 5ce6bb0

File tree

3 files changed

+16
-19
lines changed

3 files changed

+16
-19
lines changed

Diff for: library/std/src/panicking.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -253,17 +253,21 @@ 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 _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 {
261-
Some(BacktraceStyle::Short) => {
264+
// SAFETY: we took out a lock just a second ago.
265+
Some(BacktraceStyle::Short) => unsafe {
262266
drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Short))
263-
}
264-
Some(BacktraceStyle::Full) => {
267+
},
268+
Some(BacktraceStyle::Full) => unsafe {
265269
drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Full))
266-
}
270+
},
267271
Some(BacktraceStyle::Off) => {
268272
if FIRST_PANIC.swap(false, Ordering::Relaxed) {
269273
let _ = writeln!(

Diff for: library/std/src/sys/backtrace.rs

+4-11
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ use crate::sync::{Mutex, PoisonError};
1212
/// Max number of frames to print.
1313
const MAX_NB_FRAMES: usize = 100;
1414

15-
pub fn lock() -> impl Drop {
15+
pub(crate) fn lock() -> impl Drop {
1616
static LOCK: Mutex<()> = Mutex::new(());
1717
LOCK.lock().unwrap_or_else(PoisonError::into_inner)
1818
}
1919

2020
/// Prints the current backtrace.
21-
pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
21+
///
22+
/// SAFETY: this function is not Sync. The caller must hold a mutex lock, or there must be only one thread in the program.
23+
pub(crate) unsafe fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
2224
// There are issues currently linking libbacktrace into tests, and in
2325
// general during std's own unit tests we're not testing this path. In
2426
// test mode immediately return here to optimize away any references to the
@@ -27,15 +29,6 @@ pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
2729
return Ok(());
2830
}
2931

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-
}
37-
38-
unsafe fn _print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
3932
struct DisplayBacktrace {
4033
format: PrintFmt,
4134
}
+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
thread '<unnamed>' panicked at $DIR/synchronized-panic-handler.rs:thread '8<unnamed>:5' panicked at :
2-
oops oh no woe is me$DIR/synchronized-panic-handler.rs
3-
:note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
4-
8:5:
1+
thread '<unnamed>' panicked at $DIR/synchronized-panic-handler.rs:8:5:
2+
oops oh no woe is me
3+
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
4+
thread '<unnamed>' panicked at $DIR/synchronized-panic-handler.rs:8:5:
55
oops oh no woe is me

0 commit comments

Comments
 (0)