Skip to content
This repository was archived by the owner on Aug 16, 2021. It is now read-only.

Commit fb9ea0e

Browse files
alexcrichtonYamakaky
authored andcommitted
Improve handling of backtraces
* Get the library passing `cargo test` again * Only call `env::var_os` once, cache the result in a global * Move backtrace-related cfg code to one location, avoids `#[cfg]` traffic * Don't resolve backtraces until they're displayed
1 parent baee12b commit fb9ea0e

File tree

5 files changed

+131
-104
lines changed

5 files changed

+131
-104
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ default = ["backtrace", "example_generated"]
2323
example_generated = []
2424

2525
[dependencies]
26-
backtrace = { version = "0.3", optional = true }
26+
backtrace = { version = "0.3.3", optional = true }

examples/size.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,10 @@ fn main() {
2121
println!(" ErrorKind::Msg: {}", size_of_val(&msg));
2222
println!(" String: {}", size_of::<String>());
2323
println!(" State: {}", size_of::<error_chain::State>());
24-
#[cfg(feature = "backtrace")]
25-
{
26-
let state = error_chain::State {
27-
next_error: None,
28-
backtrace: None,
29-
};
30-
println!(" State.next_error: {}", size_of_val(&state.next_error));
31-
println!(" State.backtrace: {}", size_of_val(&state.backtrace));
32-
}
33-
#[cfg(not(feature = "backtrace"))]
34-
{
35-
let state = error_chain::State { next_error: None };
36-
println!(" State.next_error: {}", size_of_val(&state.next_error));
37-
}
24+
let state = error_chain::State {
25+
next_error: None,
26+
backtrace: error_chain::InternalBacktrace::new(),
27+
};
28+
println!(" State.next_error: {}", size_of_val(&state.next_error));
29+
println!(" State.backtrace: {}", size_of_val(&state.backtrace));
3830
}

src/backtrace.rs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
pub use self::imp::{Backtrace, InternalBacktrace};
2+
3+
#[cfg(feature = "backtrace")]
4+
mod imp {
5+
extern crate backtrace;
6+
7+
use std::cell::UnsafeCell;
8+
use std::env;
9+
use std::fmt;
10+
use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
11+
use std::sync::{Arc, Mutex};
12+
13+
/// Internal representation of a backtrace
14+
#[doc(hidden)]
15+
#[derive(Clone)]
16+
pub struct InternalBacktrace {
17+
backtrace: Option<Arc<MaybeResolved>>,
18+
}
19+
20+
struct MaybeResolved {
21+
resolved: Mutex<bool>,
22+
backtrace: UnsafeCell<Backtrace>,
23+
}
24+
25+
unsafe impl Send for MaybeResolved {}
26+
unsafe impl Sync for MaybeResolved {}
27+
28+
pub use self::backtrace::Backtrace;
29+
30+
impl InternalBacktrace {
31+
/// Returns a backtrace of the current call stack if `RUST_BACKTRACE`
32+
/// is set to anything but ``0``, and `None` otherwise. This is used
33+
/// in the generated error implementations.
34+
#[doc(hidden)]
35+
pub fn new() -> InternalBacktrace {
36+
static ENABLED: AtomicUsize = ATOMIC_USIZE_INIT;
37+
38+
match ENABLED.load(Ordering::SeqCst) {
39+
0 => {
40+
let enabled = match env::var_os("RUST_BACKTRACE") {
41+
Some(ref val) if val != "0" => true,
42+
_ => false,
43+
};
44+
ENABLED.store(enabled as usize + 1, Ordering::SeqCst);
45+
if !enabled {
46+
return InternalBacktrace { backtrace: None }
47+
}
48+
}
49+
1 => return InternalBacktrace { backtrace: None },
50+
_ => {}
51+
}
52+
53+
InternalBacktrace {
54+
backtrace: Some(Arc::new(MaybeResolved {
55+
resolved: Mutex::new(false),
56+
backtrace: UnsafeCell::new(Backtrace::new_unresolved()),
57+
})),
58+
}
59+
}
60+
61+
/// Acquire the internal backtrace
62+
#[doc(hidden)]
63+
pub fn as_backtrace(&self) -> Option<&Backtrace> {
64+
let bt = match self.backtrace {
65+
Some(ref bt) => bt,
66+
None => return None,
67+
};
68+
let mut resolved = bt.resolved.lock().unwrap();
69+
unsafe {
70+
if !*resolved {
71+
(*bt.backtrace.get()).resolve();
72+
*resolved = true;
73+
}
74+
Some(&*bt.backtrace.get())
75+
}
76+
}
77+
}
78+
79+
impl fmt::Debug for InternalBacktrace {
80+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
81+
f.debug_struct("InternalBacktrace")
82+
.field("backtrace", &self.as_backtrace())
83+
.finish()
84+
}
85+
}
86+
}
87+
88+
#[cfg(not(feature = "backtrace"))]
89+
mod imp {
90+
/// Dummy type used when the `backtrace` feature is disabled.
91+
pub type Backtrace = ();
92+
93+
/// Internal representation of a backtrace
94+
#[doc(hidden)]
95+
#[derive(Clone, Debug)]
96+
pub struct InternalBacktrace {}
97+
98+
impl InternalBacktrace {
99+
/// Returns a new backtrace
100+
#[doc(hidden)]
101+
pub fn new() -> InternalBacktrace {
102+
InternalBacktrace {}
103+
}
104+
105+
/// Returns the internal backtrace
106+
#[doc(hidden)]
107+
pub fn as_backtrace(&self) -> Option<&Backtrace> {
108+
None
109+
}
110+
}
111+
}

src/error_chain.rs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -419,40 +419,25 @@ macro_rules! error_chain {
419419
/// for more details.
420420
#[macro_export]
421421
#[doc(hidden)]
422-
#[cfg(feature = "backtrace")]
423422
macro_rules! impl_extract_backtrace {
424423
($error_name: ident
425424
$error_kind_name: ident
426425
$([$link_error_path: path, $(#[$meta_links: meta])*])*) => {
427426
#[allow(unknown_lints, unused_doc_comment)]
428427
fn extract_backtrace(e: &(::std::error::Error + Send + 'static))
429-
-> Option<::std::sync::Arc<$crate::Backtrace>> {
428+
-> Option<$crate::InternalBacktrace> {
430429
if let Some(e) = e.downcast_ref::<$error_name>() {
431-
return e.1.backtrace.clone();
430+
return Some(e.1.backtrace.clone());
432431
}
433432
$(
434433
$( #[$meta_links] )*
435434
{
436435
if let Some(e) = e.downcast_ref::<$link_error_path>() {
437-
return e.1.backtrace.clone();
436+
return Some(e.1.backtrace.clone());
438437
}
439438
}
440439
) *
441440
None
442441
}
443442
}
444443
}
445-
446-
/// Macro used to manage the `backtrace` feature.
447-
///
448-
/// See
449-
/// https://www.reddit.com/r/rust/comments/57virt/hey_rustaceans_got_an_easy_question_ask_here/da5r4ti/?context=3
450-
/// for more details.
451-
#[macro_export]
452-
#[doc(hidden)]
453-
#[cfg(not(feature = "backtrace"))]
454-
macro_rules! impl_extract_backtrace {
455-
($error_name: ident
456-
$error_kind_name: ident
457-
$([$link_error_path: path, $(#[$meta_links: meta])*])*) => {}
458-
}

src/lib.rs

Lines changed: 10 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -536,22 +536,10 @@
536536
//! [`map_err`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_err
537537
//! [`BacktraceFrame`]: https://docs.rs/backtrace/0.3.2/backtrace/struct.BacktraceFrame.html
538538
539-
540-
#[cfg(feature = "backtrace")]
541-
extern crate backtrace;
542-
543539
use std::error;
544540
use std::iter::Iterator;
545-
#[cfg(feature = "backtrace")]
546-
use std::sync::Arc;
547541
use std::fmt;
548542

549-
#[cfg(feature = "backtrace")]
550-
pub use backtrace::Backtrace;
551-
#[cfg(not(feature = "backtrace"))]
552-
/// Dummy type used when the `backtrace` feature is disabled.
553-
pub type Backtrace = ();
554-
555543
#[macro_use]
556544
mod impl_error_chain_kind;
557545
#[macro_use]
@@ -561,6 +549,10 @@ mod quick_main;
561549
pub use quick_main::ExitCode;
562550
#[cfg(feature = "example_generated")]
563551
pub mod example_generated;
552+
mod backtrace;
553+
pub use backtrace::Backtrace;
554+
#[doc(hidden)]
555+
pub use backtrace::InternalBacktrace;
564556

565557
#[derive(Debug)]
566558
/// Iterator over the error chain using the `Error::cause()` method.
@@ -587,38 +579,6 @@ impl<'a> Iterator for Iter<'a> {
587579
}
588580
}
589581

590-
/// Returns a backtrace of the current call stack if `RUST_BACKTRACE`
591-
/// is set to anything but ``0``, and `None` otherwise. This is used
592-
/// in the generated error implementations.
593-
#[cfg(feature = "backtrace")]
594-
#[doc(hidden)]
595-
pub fn make_backtrace() -> Option<Arc<Backtrace>> {
596-
use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering};
597-
598-
// The lowest bit indicates whether the value was computed,
599-
// while the second lowest bit is the actual "enabled" bit.
600-
static BACKTRACE_ENABLED_CACHE: AtomicUsize = ATOMIC_USIZE_INIT;
601-
602-
let enabled = match BACKTRACE_ENABLED_CACHE.load(Ordering::Relaxed) {
603-
0 => {
604-
let enabled = match std::env::var_os("RUST_BACKTRACE") {
605-
Some(ref val) if val != "0" => true,
606-
_ => false
607-
};
608-
let encoded = ((enabled as usize) << 1) | 1;
609-
BACKTRACE_ENABLED_CACHE.store(encoded, Ordering::Relaxed);
610-
enabled
611-
}
612-
encoded => (encoded >> 1) != 0
613-
};
614-
615-
if enabled {
616-
Some(Arc::new(Backtrace::new()))
617-
} else {
618-
None
619-
}
620-
}
621-
622582
/// This trait is implemented on all the errors generated by the `error_chain`
623583
/// macro.
624584
pub trait ChainedError: error::Error + Send + 'static {
@@ -662,9 +622,8 @@ pub trait ChainedError: error::Error + Send + 'static {
662622

663623
/// Returns the first known backtrace, either from its State or from one
664624
/// of the errors from `foreign_links`.
665-
#[cfg(feature = "backtrace")]
666625
#[doc(hidden)]
667-
fn extract_backtrace(e: &(error::Error + Send + 'static)) -> Option<Arc<Backtrace>>
626+
fn extract_backtrace(e: &(error::Error + Send + 'static)) -> Option<InternalBacktrace>
668627
where Self: Sized;
669628
}
670629

@@ -698,52 +657,32 @@ pub struct State {
698657
/// Next error in the error chain.
699658
pub next_error: Option<Box<error::Error + Send>>,
700659
/// Backtrace for the current error.
701-
#[cfg(feature = "backtrace")]
702-
pub backtrace: Option<Arc<Backtrace>>,
660+
pub backtrace: InternalBacktrace,
703661
}
704662

705663
impl Default for State {
706-
#[cfg(feature = "backtrace")]
707664
fn default() -> State {
708665
State {
709666
next_error: None,
710-
backtrace: make_backtrace(),
667+
backtrace: InternalBacktrace::new(),
711668
}
712669
}
713-
714-
#[cfg(not(feature = "backtrace"))]
715-
fn default() -> State {
716-
State { next_error: None }
717-
}
718670
}
719671

720672
impl State {
721673
/// Creates a new State type
722-
#[cfg(feature = "backtrace")]
723674
pub fn new<CE: ChainedError>(e: Box<error::Error + Send>) -> State {
724-
let backtrace = CE::extract_backtrace(&*e).or_else(make_backtrace);
675+
let backtrace = CE::extract_backtrace(&*e)
676+
.unwrap_or_else(InternalBacktrace::new);
725677
State {
726678
next_error: Some(e),
727679
backtrace: backtrace,
728680
}
729681
}
730682

731-
/// Creates a new State type
732-
#[cfg(not(feature = "backtrace"))]
733-
pub fn new<CE: ChainedError>(e: Box<error::Error + Send>) -> State {
734-
State { next_error: Some(e) }
735-
}
736-
737-
/// Returns the inner backtrace if present.
738-
#[cfg(feature = "backtrace")]
739-
pub fn backtrace(&self) -> Option<&Backtrace> {
740-
self.backtrace.as_ref().map(|v| &**v)
741-
}
742-
743683
/// Returns the inner backtrace if present.
744-
#[cfg(not(feature = "backtrace"))]
745684
pub fn backtrace(&self) -> Option<&Backtrace> {
746-
None
685+
self.backtrace.as_backtrace()
747686
}
748687
}
749688

0 commit comments

Comments
 (0)