Tracking Issue for panic_backtrace_config

[Mark-Simulacrum]

Feature gate: #![feature(panic_backtrace_config)]

This is a tracking issue for configuring the capture and display of backtraces in the default panic hook, as well as exposing that configuration to third-party libraries for usage.

Public API

mod panic {
    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
    #[non_exhaustive]
    pub enum BacktraceStyle {
        Short,
        Full,
        Off,
    }
    fn set_backtrace_style(BacktraceStyle);
    fn get_backtrace_style() -> Option<BacktraceStyle>;
}

This API is intended to form part of the strategy for addressing the unsoundness of the std::env::set_var API on some platforms (mostly non-Windows). See #92431 (comment) for a summary.

Steps / History

• Implementation: Support configuring whether to capture backtraces at runtime #93101
• Final comment period (FCP)
• Stabilization PR

Unresolved Questions

• Do we need to move to a thread-local or otherwise more customizable strategy for whether to capture backtraces? See this comment for some potential use cases for this.
  • Proposed answer: no, leave this for third-party hooks.
• Bikeshed on naming of all the options, as usual.
• Should BacktraceStyle be moved into std::backtrace?
  • It's already somewhat annoying to import and/or re-type the std::panic:: prefix necessary to use these APIs, probably adding a second module to the mix isn't worth it.
• Should get_backtrace_style be exposed?
  • The Option<BacktraceStyle> return type looks a little weird, and may not mean the intuitive thing -- None represents Unsupported, not "not set" or "don't print backtraces", as might be initially assumed.
  • We may want a std::panic::report_backtrace(&mut dyn Write) instead which internally knows how to format things.

[yaahc]

• It's already somewhat annoying to import and/or re-type the std::panic:: prefix necessary to use these APIs, probably adding a second module to the mix isn't worth it.

I think it depends on how we expose the Short and Full format displays to users. If we expect users to use fmt flags to configure the format when printing std::backtrace::Backtrace then we'd probably want to associate the style with the panic module.

I do worry though about how this style of API will look like in practice, I expect you'd probably have to write something like this every time you want to copy the std panic hook behavior:

match get_backtrace_style() {
    Short => // use correct fmt flags for short
    Full => // use correct fmt flags for full
    // ...
}

Which feels a little unfortunate. This is a strawman proposal but I would personally rather use something that can be easily expressed in one line, first idea I can come up with is this:

println!("{}", backtrace.display(get_backtrace_style()));

And if we were to provide an adapter API for associating a style with a backtrace when printing it then I imagine we'd want to include the style type in the backtrace module.

[Mark-Simulacrum]

For the trivial "just print it" case, I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better (or takes a &mut dyn Write, maybe) -- capturing and resolving the backtrace via std::backtrace::Backtrace in panic handling may want to e.g. avoid allocations and some of the other complexity associated with a more general API like std::backtrace::Backtrace.

If you're doing something more complicated that doesn't just want to adjust the surrounding material but alter the backtrace printing directly, it may need some API design if we don't wan to stabilize some fairly low-level APIs (e.g., https://docs.rs/backtrace/latest/backtrace/fn.trace.html and similar) that may not be a good fit for std itself.

I'm not sure there's a great answer here, though; I think the tradeoffs are particularly acute around this surface area and difficult to judge.

[yaahc]

For the trivial "just print it" case, I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better

agreed, I've felt like we should be exposing the allocation-lite backtrace printing interface we use in the panic hook for a while now.

If you're doing something more complicated that doesn't just want to adjust the surrounding material but alter the backtrace printing directly, it may need some API design if we don't wan to stabilize some fairly low-level APIs (e.g., https://docs.rs/backtrace/latest/backtrace/fn.trace.html and similar) that may not be a good fit for std itself.

oh interesting, I can see us going this direction, though the usage of trace seems somewhat opaque / complex so like you said we'd need some design discussion on that or some more informative examples. For now I am a big enough fan of the split you describe and think we should plan on keeping BacktraceStyle in panic and add a std::panic::report_backtrace(&mut dyn Write) and let it read the backtrace style internally.

Thinking about it though, this approach definitely reduces the value of the get_backtrace_style function, since it's not really making it easy to copy the behavior of the panic hook when printing std::backtrace::Backtraces, and you wouldn't even need to use it when working with report_backtrace.

[RalfJung]

@rust-lang/libs-api I'd like to propose to stabilize these APIs as-is. The API matches what we have done with environment variables for a long time, just a bit more type-safe. There's more that can be done here (see the posts by @yaahc above), but I don't think any of this should be a blocker -- given in particular that this feature is on the critical path for reducing the use of set_var in the ecosystem.

[m-ou-se]

set_backtrace_style() is almost the same as setting RUST_BACKTRACE, except that it doesn't enable Backtrace::capture(), right? Should it? Or is that intentional?

[RalfJung]

Oh, interesting. On the one hand it seems strange if a std::panic function affects std::backtrace behavior, on the other hand ideally this function can replace the environment variable entirely.

Maybe the enum and methods should be moved to std::backtrace?

[target-san]

I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better

Sorry for interfering, though I'd suggest previous Backtrace::display(...) API for one simple reason - ability to gather backtrace and then print it using builtin styles elsewhere. It's a bit inconvenient that you either use default default panic hook and get nice short stacktraces or have to emulate it by hand for std::backtrace::Backtrace.

[tbu-]

Usually, we omit get_ from getters. So it'd be called std::panic::backtrace_style and not std::panic::get_backtrace_style.

[RalfJung]

We are getting closer to set_var being unsafe. It would be good to have this stable soon so that people can migrate away from a soon-to-be-unsafe API to something safe.

[RalfJung]

@m-ou-se

set_backtrace_style() is almost the same as setting RUST_BACKTRACE, except that it doesn't enable Backtrace::capture(), right? Should it? Or is that intentional?

What does t-libs-api think is the better choice here? I don't have a strong opinion, but this API should ideally land in time for the edition.

1. Currently implemented: std::panic::set_backtrace_style only affects panics, but not Backtrace::capture. Doesn't provide a full replacement for set_var("RUST_BACKTRACE", _), but is probably good enough.
2. Have std::panic::set_backtrace_style also affect Backtrace::capture. That's a somewhat odd coupling of module.
3. Move set_backtrace_style (and get_backtrace_style) to std::backtrace, to indicate that they affect all ways of capturing backtraces (via Backtrace and inside panics). However, backtrace doesn't really make a difference between "short" and "full" so this is also a bit awkward.

Cc @rust-lang/libs-api

[Amanieu]

We discussed this in the libs-api meeting today. The main point of contention is that we have 2 separate points where we configure the "backtrace mode": RUST_BACKTRACE and RUST_LIB_BACKTRACE. There are valid use cases for configuring both:

• RUST_LIB_BACKTRACE can be turned off to reduce the performance impact of backtrace capture in crates like anyhow.
• RUST_BACKTRACE can be turned on to always provide users with a backtrace for bug reports.

Our conclusion is that we should provide APIs to individually control both of these options:

// std::panic
fn set_panic_backtrace_style(style: BacktraceStyle);
fn panic_backtrace_style() -> BacktraceStyle;

// std::backtrace
fn set_backtrace_style(style: BacktraceStyle);
fn backtrace_style() -> BacktraceStyle;

Some notable differences between this API and the existing one:

• backtrace_style doesn't return an Option. Missing backtrace support is an implementation detail that is only reported when actually trying to obtain a backtrace. The backtrace style API ignores this and only sets the "desired" panic style, independently of whether it is supported.
• The 2 options are independent of each other: one is for the default panic hook, the other is for Backtrace::capture. Each can be set independently and one does not affect the other.
• If the style is not set then it will be initialized from environment variables only. Notably the default value for backtrace_style will not reflect any previous calls to set_panic_backtrace_style.

[RalfJung]

@Amanieu thanks!

Will these use the same BacktraceStyle type? AFAIK, std::backtrace only supports "full" and "no" capture; there's no such thing as "short" there?

[3tilley]

My understanding is that the consideration of making this possible in a thread-local manner has been pushed to third party libraries - meaning there is confidence that the standard library solution would leave sufficient hooks for third party libraries to do this efficiently - is that correct?

Is that also how this would be handled in an async context? A good usecase is should_panic tests where someone might disable backtraces for performance reasons, but only for those tests. They may be running in different threads, or might be async tests. Would that be easily handled under the current proposal (even if it's delegated to third party libraries)?