diff --git a/Cargo.toml b/Cargo.toml index 219a17eb9..681972042 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,14 +67,14 @@ std = [] # - unix-backtrace: this uses the backtrace(3) function to acquire a # backtrace, but is not as reliable as libunwind. It is, however, # generally found in more locations. -# - dbghelp: on windows this enables usage of dbghelp.dll to find a -# backtrace at runtime +# - dbghelp: on windows this enables usage of dbghelp.dll to acquire and +# resolve a backtrace at runtime # - kernel32: on windows this enables using RtlCaptureStackBackTrace as the # function to acquire a backtrace libunwind = [] unix-backtrace = [] -dbghelp = ["std"] -kernel32 = ["dbghelp"] +dbghelp = [] +kernel32 = [] #======================================= # Methods of resolving symbols diff --git a/ci/azure-test-all.yml b/ci/azure-test-all.yml index b2b7124d3..6c334bead 100644 --- a/ci/azure-test-all.yml +++ b/ci/azure-test-all.yml @@ -37,5 +37,9 @@ steps: displayName: "Test backtrace (-default + gimli-symbolize + std)" - bash: cargo test --no-default-features --features 'dbghelp std' displayName: "Test backtrace (-default + dbghelp + std)" + - bash: cargo test --no-default-features --features 'kernel32 std' + displayName: "Test backtrace (-default + kernel32 + std)" + - bash: cargo test --no-default-features --features 'kernel32 dbghelp std' + displayName: "Test backtrace (-default + kernel32 + dbghelp + std)" - bash: cd ./cpp_smoke_test && cargo test displayName: "Test cpp_smoke_test" diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index 7c05b8172..84e981316 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -62,7 +62,7 @@ pub unsafe fn trace(cb: &mut FnMut(&super::Frame) -> bool) { }; let image = init_frame(&mut frame.inner.inner, &context.0); - let cleanup_on_drop = CleanupOnDrop; + let _cleanup_on_drop = CleanupOnDrop; ::TRACE_CLEANUP.with(|trace_cleanup| { let mut trace_cleanup = trace_cleanup.borrow_mut(); @@ -120,8 +120,6 @@ pub unsafe fn trace(cb: &mut FnMut(&super::Frame) -> bool) { break } } - - drop(cleanup_on_drop); } } diff --git a/src/capture.rs b/src/capture.rs index 6c1da2773..90d370425 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -68,6 +68,8 @@ impl Backtrace { /// ``` #[inline(never)] // want to make sure there's a frame here to remove pub fn new() -> Backtrace { + let _lock = ::lock::lock(); + // initialize dbghelp only once for both the trace and the resolve #[cfg(all(windows, feature = "dbghelp"))] let _c = unsafe { ::dbghelp_init() }; @@ -145,6 +147,8 @@ impl Backtrace { /// If this backtrace has been previously resolved or was created through /// `new`, this function does nothing. pub fn resolve(&mut self) { + let _lock = ::lock::lock(); + // initialize dbghelp only once for all frames #[cfg(all(windows, feature = "dbghelp"))] let _c = unsafe { ::dbghelp_init() }; diff --git a/src/lib.rs b/src/lib.rs index 92067cd34..5babd4190 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -180,29 +180,18 @@ mod lock { } #[cfg(all(windows, feature = "dbghelp"))] -struct Cleanup { - handle: winapi::um::winnt::HANDLE, - opts: winapi::shared::minwindef::DWORD, -} +mod dbghelp { + use core::marker::PhantomData; + use winapi::um::dbghelp::{SymInitializeW, SymCleanup}; -#[cfg(all(windows, feature = "dbghelp"))] -enum Trace { - Inside(Option), - Outside, -} - -thread_local! { - #[cfg(all(windows, feature = "dbghelp"))] - static TRACE_CLEANUP: std::cell::RefCell = std::cell::RefCell::new(Trace::Outside); -} + // Not sure why these are missing in winapi -#[cfg(all(windows, feature = "dbghelp"))] -unsafe fn dbghelp_init() -> Option { - use winapi::shared::minwindef; - use winapi::um::{dbghelp, processthreadsapi}; + const SYMOPT_DEFERRED_LOADS: winapi::shared::minwindef::DWORD = 0x00000004; - use std::sync::{Mutex, Once, ONCE_INIT}; - use std::boxed::Box; + extern "system" { + fn SymGetOptions() -> winapi::shared::minwindef::DWORD; + fn SymSetOptions(options: winapi::shared::minwindef::DWORD); + } // Initializing symbols has significant overhead, but initializing only once // without cleanup causes problems for external sources. For example, the @@ -214,68 +203,84 @@ unsafe fn dbghelp_init() -> Option { // As a compromise, we'll keep track of the number of internal initialization // requests within a single API call in order to minimize the number of // init/cleanup cycles. - static mut REF_COUNT: *mut Mutex = 0 as *mut _; - static mut INIT: Once = ONCE_INIT; + static mut DBGHELP: DbgHelp = DbgHelp { + ref_count: 0, + opts: 0, + handle: core::ptr::null_mut() + }; + + struct DbgHelp { + ref_count: usize, + handle: winapi::um::winnt::HANDLE, + opts: winapi::shared::minwindef::DWORD, + } - INIT.call_once(|| { - REF_COUNT = Box::into_raw(Box::new(Mutex::new(0))); - }); + pub struct Cleanup(PhantomData<*mut ()>); - // Not sure why these are missing in winapi - const SYMOPT_DEFERRED_LOADS: minwindef::DWORD = 0x00000004; - extern "system" { - fn SymGetOptions() -> minwindef::DWORD; - fn SymSetOptions(options: minwindef::DWORD); + impl Clone for Cleanup { + fn clone(&self) -> Cleanup { + unsafe { + DBGHELP.ref_count += 1; + } + Cleanup(PhantomData) + } } impl Drop for Cleanup { fn drop(&mut self) { unsafe { - let mut ref_count_guard = (&*REF_COUNT).lock().unwrap(); - *ref_count_guard -= 1; - - if *ref_count_guard == 0 { - dbghelp::SymCleanup(self.handle); - SymSetOptions(self.opts); + DBGHELP.ref_count -= 1; + if DBGHELP.ref_count == 0 { + SymCleanup(DBGHELP.handle); + SymSetOptions(DBGHELP.opts); } } } } - impl Clone for Cleanup { - fn clone(&self) -> Cleanup { - unsafe { - let mut ref_count_guard = (&*REF_COUNT).lock().unwrap(); - *ref_count_guard += 1; - } - Cleanup { - opts: self.opts, - handle: self.handle - } - } + /// Tracks whether or not we have initialized dbghelp.dll inside of a call to `trace`. + pub enum Trace { + Inside(Option), + Outside, } - let opts = SymGetOptions(); - let handle = processthreadsapi::GetCurrentProcess(); + thread_local! { + pub static TRACE_CLEANUP: core::cell::RefCell = core::cell::RefCell::new(Trace::Outside); + } - let mut ref_count_guard = (&*REF_COUNT).lock().unwrap(); + /// Requires external synchronization + pub unsafe fn init() -> Option { + use winapi::shared::minwindef::TRUE; + use winapi::um::processthreadsapi::GetCurrentProcess; - if *ref_count_guard > 0 { - *ref_count_guard += 1; - return Some(Cleanup { handle, opts }); - } + if DBGHELP.ref_count > 0 { + DBGHELP.ref_count += 1; + return Some(Cleanup(core::marker::PhantomData)); + } + + let opts = SymGetOptions(); + let handle = GetCurrentProcess(); - SymSetOptions(opts | SYMOPT_DEFERRED_LOADS); + SymSetOptions(opts | SYMOPT_DEFERRED_LOADS); - let ret = dbghelp::SymInitializeW(handle, - 0 as *mut _, - minwindef::TRUE); + let ret = SymInitializeW(handle, 0 as *mut _,TRUE); + if ret != TRUE { + // Revert our changes + SymSetOptions(opts); - if ret != minwindef::TRUE { - // Symbols may have been initialized by another library or an external debugger - None - } else { - *ref_count_guard += 1; - Some(Cleanup { handle, opts }) + // Symbols may have been initialized by another library or an external debugger + None + } else { + DBGHELP.ref_count += 1; + DBGHELP.handle = handle; + DBGHELP.opts = opts; + Some(Cleanup(PhantomData)) + } } -} \ No newline at end of file +} + +#[cfg(all(windows, feature = "dbghelp"))] +use dbghelp::init as dbghelp_init; + +#[cfg(all(windows, feature = "dbghelp"))] +use dbghelp::{Trace, TRACE_CLEANUP};