about summary refs log tree commit diff
path: root/library/std/src/sys
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-07-13 03:42:24 +0000
committerbors <bors@rust-lang.org>2024-07-13 03:42:24 +0000
commit0065384608dfb27695f50e709d9689a7f4d0cdbc (patch)
tree367fa16854e45e8fc29c74011a216c74c72d7846 /library/std/src/sys
parent03c2100dede5d8779a032cf1fc1c28201c37048a (diff)
parent1c8f9bb84d5956abe6671bf77ccaba64505266a5 (diff)
Auto merge of #127397 - jyn514:multi-thread-panic-hook, r=workingjubilee
fix interleaved output in the default panic hook when multiple threads panic simultaneously

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 from one thread would be interleaved with the backtrace from another. now, we hold the lock for the full duration of the hook, and the output is ordered.

---

i noticed some odd things while working on this you may or may not already be aware of.

- libbacktrace is included as a submodule instead of a normal rustc crate, and as a result uses `cfg(backtrace_in_std)` instead of a more normal `cfg(feature = "rustc-dep-of-std")`. probably this is left over from before rust used a cargo-based build system?
- the default panic handler uses `trace_unsynchronized`, etc, in `sys::backtrace::print`. as a result, the lock only applies to concurrent *panic handlers*, not concurrent *threads*.  in other words, if another, non-panicking, thread tried to print a backtrace at the same time as the panic handler, we may have UB, especially on windows.
    - we have the option of changing backtrace to enable locking when `backtrace_in_std` is set so we can reuse their lock instead of trying to add our own.
Diffstat (limited to 'library/std/src/sys')
-rw-r--r--library/std/src/sys/backtrace.rs53
1 files changed, 25 insertions, 28 deletions
diff --git a/library/std/src/sys/backtrace.rs b/library/std/src/sys/backtrace.rs
index 0b2338fd7de..7401d8ce320 100644
--- a/library/std/src/sys/backtrace.rs
+++ b/library/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 {