From 39647aba92c3f8620a7defe428e8f2d5e7c70ecc Mon Sep 17 00:00:00 2001
From: jyn <github@jyn.dev>
Date: Fri, 5 Jul 2024 17:24:10 -0400
Subject: [PATCH] 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.
---
 std/src/panicking.rs     |  8 ++++--
 std/src/sys/backtrace.rs | 53 +++++++++++++++++++---------------------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/std/src/panicking.rs b/std/src/panicking.rs
index ebd054156951d..418a855fb728e 100644
--- a/std/src/panicking.rs
+++ b/std/src/panicking.rs
@@ -253,16 +253,20 @@ fn default_hook(info: &PanicHookInfo<'_>) {
     let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");
 
     let write = |err: &mut dyn crate::io::Write| {
+        // Use a lock to prevent mixed output in multithreading context.
+        // Some platforms also require it when printing a backtrace, like `SymFromAddr` on Windows.
+        let mut lock = backtrace::lock();
         let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}");
 
         static FIRST_PANIC: AtomicBool = AtomicBool::new(true);
 
         match backtrace {
+            // SAFETY: we took out a lock just a second ago.
             Some(BacktraceStyle::Short) => {
-                drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Short))
+                drop(lock.print(err, crate::backtrace_rs::PrintFmt::Short))
             }
             Some(BacktraceStyle::Full) => {
-                drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Full))
+                drop(lock.print(err, crate::backtrace_rs::PrintFmt::Full))
             }
             Some(BacktraceStyle::Off) => {
                 if FIRST_PANIC.swap(false, Ordering::Relaxed) {
diff --git a/std/src/sys/backtrace.rs b/std/src/sys/backtrace.rs
index 0b2338fd7de9b..7401d8ce32087 100644
--- a/std/src/sys/backtrace.rs
+++ b/std/src/sys/backtrace.rs
@@ -7,44 +7,41 @@ use crate::fmt;
 use crate::io;
 use crate::io::prelude::*;
 use crate::path::{self, Path, PathBuf};
-use crate::sync::{Mutex, PoisonError};
+use crate::sync::{Mutex, MutexGuard, PoisonError};
 
 /// Max number of frames to print.
 const MAX_NB_FRAMES: usize = 100;
 
-pub fn lock() -> impl Drop {
+pub(crate) struct BacktraceLock<'a>(#[allow(dead_code)] MutexGuard<'a, ()>);
+
+pub(crate) fn lock<'a>() -> BacktraceLock<'a> {
     static LOCK: Mutex<()> = Mutex::new(());
-    LOCK.lock().unwrap_or_else(PoisonError::into_inner)
+    BacktraceLock(LOCK.lock().unwrap_or_else(PoisonError::into_inner))
 }
 
-/// Prints the current backtrace.
-pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
-    // There are issues currently linking libbacktrace into tests, and in
-    // general during std's own unit tests we're not testing this path. In
-    // test mode immediately return here to optimize away any references to the
-    // libbacktrace symbols
-    if cfg!(test) {
-        return Ok(());
-    }
-
-    // Use a lock to prevent mixed output in multithreading context.
-    // Some platforms also requires it, like `SymFromAddr` on Windows.
-    unsafe {
-        let _lock = lock();
-        _print(w, format)
-    }
-}
+impl BacktraceLock<'_> {
+    /// Prints the current backtrace.
+    ///
+    /// NOTE: this function is not Sync. The caller must hold a mutex lock, or there must be only one thread in the program.
+    pub(crate) fn print(&mut self, w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
+        // There are issues currently linking libbacktrace into tests, and in
+        // general during std's own unit tests we're not testing this path. In
+        // test mode immediately return here to optimize away any references to the
+        // libbacktrace symbols
+        if cfg!(test) {
+            return Ok(());
+        }
 
-unsafe fn _print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
-    struct DisplayBacktrace {
-        format: PrintFmt,
-    }
-    impl fmt::Display for DisplayBacktrace {
-        fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-            unsafe { _print_fmt(fmt, self.format) }
+        struct DisplayBacktrace {
+            format: PrintFmt,
+        }
+        impl fmt::Display for DisplayBacktrace {
+            fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+                unsafe { _print_fmt(fmt, self.format) }
+            }
         }
+        write!(w, "{}", DisplayBacktrace { format })
     }
-    write!(w, "{}", DisplayBacktrace { format })
 }
 
 unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::Result {