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.

Sign up for GitHub

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

Add Backtrace::set_enabled to override environment variables #79085

Closed
wants to merge 1 commit into from
Closed

Add Backtrace::set_enabled to override environment variables #79085

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions library/std/src/backtrace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,15 @@ impl fmt::Debug for BytesOrWide {
}
}

static ENABLED: AtomicUsize = AtomicUsize::new(0);

impl Backtrace {
/// Returns whether backtrace captures are enabled through environment
/// variables.
fn enabled() -> bool {
// Cache the result of reading the environment variables to make
// backtrace captures speedy, because otherwise reading environment
// variables every time can be somewhat slow.
static ENABLED: AtomicUsize = AtomicUsize::new(0);
match ENABLED.load(SeqCst) {
0 => {}
1 => return false,
Expand All @@ -251,10 +252,17 @@ impl Backtrace {
Err(_) => false,
},
};
ENABLED.store(enabled as usize + 1, SeqCst);
Self::set_enabled(enabled);
enabled
}

/// Explicitly enable or disable backtrace capturing, overriding the default
/// derived from the `RUST_BACKTRACE` or `RUST_LIB_BACKTRACE` environment
/// variables.
pub fn set_enabled(enabled: bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put an unstable attribute on this, since we've got #72981 open to stabilize the core backtrace API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API doesn't look sufficient enough to handle the full functionality of what RUST_BACKTRACE can do, as RUST_BACKTRACE has multiple values, and might even be extended to more in the future. I think it would be good to at least change this to take an enum, something like this:

#[non_exhaustive]
pub Enum {
    Off,
    Short,
    Full,
}

Also, I think that this API should be "set at most once," similar to log::set_logger, as an application like the one I maintain doesn't want a (third-party) library to change this value after we set it in main(). (The ability to lock the backtrace level is another motivation for adding this new API.)

ENABLED.store(enabled as usize + 1, SeqCst);
}

/// Capture a stack backtrace of the current thread.
///
/// This function will capture a stack backtrace of the current OS thread of
Expand Down