Auto merge of #24890 - servo:signal-deadlock, r=jdm

Avoid allocation in the signal handler to fix a deadlock

Fixes https://github.com/servo/servo/issues/24881
CC https://github.com/rust-lang/backtrace-rs/pull/265

2 files changed, 115 insertions, 8 deletions

diff --git a/ports/glutin/backtrace.rs b/ports/glutin/backtrace.rs
new file mode 100644
index 00000000000..6e5a8390042
--- /dev/null
+++ b/ports/glutin/backtrace.rs
@@ -0,0 +1,103 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+//! Similar to `println!("{:?}", Backtrace::new())`, but doesn’t allocate.
+//!
+//! Seems to fix some deadlocks: https://github.com/servo/servo/issues/24881
+//!
+//! FIXME: if/when a future version of the `backtrace` crate has
+//! https://github.com/rust-lang/backtrace-rs/pull/265, use that instead.
+
+use std::fmt::{self, Write};
+use backtrace::{BytesOrWideString, PrintFmt};
+
+#[inline(never)]
+pub(crate) fn print() {
+    println!("{:?}", Print {
+        print_fn_address: print as usize,
+    })
+}
+
+struct Print {
+    print_fn_address: usize,
+}
+
+impl fmt::Debug for Print {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        // Safety: we’re in a signal handler that is about to call `libc::_exit`.
+        // Potential data races from using `*_unsynchronized` functions are perhaps
+        // less bad than potential deadlocks?
+        unsafe {
+            let mut print_fn_frame = 0;
+            let mut frame_count = 0;
+            backtrace::trace_unsynchronized(|frame| {
+                let found = frame.symbol_address() as usize == self.print_fn_address;
+                if found {
+                    print_fn_frame = frame_count;
+                }
+                frame_count += 1;
+                !found
+            });
+
+            let mode = PrintFmt::Short;
+            let mut p = print_path;
+            let mut f = backtrace::BacktraceFmt::new(fmt, mode, &mut p);
+            f.add_context()?;
+            let mut result = Ok(());
+            let mut frame_count = 0;
+            backtrace::trace_unsynchronized(|frame| {
+                let skip = frame_count < print_fn_frame;
+                frame_count += 1;
+                if skip {
+                    return true
+                }
+
+                let mut frame_fmt = f.frame();
+                let mut any_symbol = false;
+                backtrace::resolve_frame_unsynchronized(frame, |symbol| {
+                    any_symbol = true;
+                    if let Err(e) = frame_fmt.symbol(frame, symbol) {
+                        result = Err(e)
+                    }
+                });
+                if !any_symbol {
+                    if let Err(e) = frame_fmt.print_raw(frame.ip(), None, None, None) {
+                        result = Err(e)
+                    }
+                }
+                result.is_ok()
+            });
+            result?;
+            f.finish()
+        }
+    }
+}
+
+fn print_path(fmt: &mut fmt::Formatter, path: BytesOrWideString) -> fmt::Result {
+    match path {
+        BytesOrWideString::Bytes(mut bytes) => {
+            loop {
+                match std::str::from_utf8(bytes) {
+                    Ok(s) => {
+                        fmt.write_str(s)?;
+                        break;
+                    }
+                    Err(err) => {
+                        fmt.write_char(std::char::REPLACEMENT_CHARACTER)?;
+                        match err.error_len() {
+                            Some(len) => bytes = &bytes[err.valid_up_to() + len..],
+                            None => break,
+                        }
+                    }
+                }
+            }
+        }
+        BytesOrWideString::Wide(wide) => {
+            for c in std::char::decode_utf16(wide.iter().cloned()) {
+                fmt.write_char(c.unwrap_or(std::char::REPLACEMENT_CHARACTER))?
+            }
+        }
+    }
+    Ok(())
+}
diff --git a/ports/glutin/main2.rs b/ports/glutin/main2.rs
index b8d784db762..630ccaf8945 100644
--- a/ports/glutin/main2.rs
+++ b/ports/glutin/main2.rs
@@ -11,6 +11,7 @@ extern crate log;
 extern crate sig;
 
 mod app;
+mod backtrace;
 mod browser;
 mod context;
 mod embedder;
@@ -23,7 +24,6 @@ mod skia_symbols;
 mod window_trait;
 
 use app::App;
-use backtrace::Backtrace;
 use getopts::Options;
 use servo::config::opts::{self, ArgumentParsingResult};
 use servo::config::servo_version;
@@ -49,17 +49,21 @@ fn install_crash_handler() {}
 
 #[cfg(any(target_os = "macos", target_os = "linux"))]
 fn install_crash_handler() {
-    use backtrace::Backtrace;
     use libc::_exit;
     use sig::ffi::Sig;
     use std::thread;
 
     extern "C" fn handler(sig: i32) {
-        let name = thread::current()
-            .name()
-            .map(|n| format!(" for thread \"{}\"", n))
-            .unwrap_or("".to_owned());
-        println!("Stack trace{}\n{:?}", name, Backtrace::new());
+        use std::sync::atomic;
+        static BEEN_HERE_BEFORE: atomic::AtomicBool = atomic::AtomicBool::new(false);
+        if !BEEN_HERE_BEFORE.swap(true, atomic::Ordering::SeqCst) {
+            print!("Stack trace");
+            if let Some(name) = thread::current().name() {
+                print!(" for thread \"{}\"", name);
+            }
+            println!();
+            backtrace::print();
+        }
         unsafe {
             _exit(sig);
         }
@@ -139,7 +143,7 @@ pub fn main() {
             println!("{} (thread {})", msg, name);
         }
         if env::var("RUST_BACKTRACE").is_ok() {
-            println!("{:?}", Backtrace::new());
+            backtrace::print();
         }
 
         error!("{}", msg);