Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid allocation in the signal handler to fix a deadlock #24890

Merged
merged 3 commits into from Nov 27, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -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(())
}
@@ -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);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.