Skip to content

Add multithreading support for libbacktrace #2

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

Merged
merged 3 commits into from
Jul 16, 2015

Conversation

sujayakar
Copy link

Our previous usage of libbacktrace wasn't thread-safe since it passed threaded = 0 to backtrace_create_state, which then requires all library calls to be serialized. It's pretty easy to get a segfault by calling into the backtrace library across multiple threads:

#![feature(drain)]
extern crate backtrace;

use std::str;
use std::thread;

fn iter() {
    for _ in (0..16) {
        backtrace::trace(&mut |frame| {
            backtrace::resolve(frame.ip(), &mut |symbol| {
                symbol.name().and_then(|s| str::from_utf8(s).ok()).map(|name| {
                    let mut demangled = String::new();
                    backtrace::demangle(&mut demangled, name).unwrap();
                    println!("{}", demangled);
                });
            });
            true
        });
    }
}

fn main() {
    let mut threads: Vec<_> = (0..16)
        .map(|_| thread::spawn(iter))
        .collect();

    for t in threads.drain(..) {
        t.join().unwrap()
    }
}

This patch adds a StaticMutex fallback for when we detect that libbacktrace isn't thread-safe.

@sujayakar
Copy link
Author

hmm, @alexcrichton what's the recommended set of feature flags if we want this to build on stable, release, & nightly?

@alexcrichton
Copy link
Member

Oh wow this totally slipped my mind! Manually synchronizing has the downside of possibly deadlocking if it's called recursively, but I feel like that's fine

@alexcrichton
Copy link
Member

Er oops, commented too soon.

Anyway, I feel like adding a mutex is fine. I think it's fine to just always request the not-thread-safe state and don't test for support built in (e.g. always grab the lock). Other than that to build on stable it basically needs to avoid StaticMutex, and I've done that in the past with a static *mut Mutex<()> initialized to 0 as *mut _ which is then dynamically initialized in a Once with a Box<Mutex<()>>. This leaks the Box but that's pretty minor so it probably doesn't matter too much.

Thanks for this! I can get around to these changes later tonight as well if you don't beat me to them :)

@alexcrichton
Copy link
Member

Oh wow I totally forgot, but to acquire a lock this lib actually already has the support, so you should just need:

let _guard = ::lock::lock();

at the top of the function. It'd also be great to have a test for this, but I can add that later as well :)

@alexcrichton alexcrichton merged commit b3fb12a into rust-lang:master Jul 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants