Skip to content

ACP: panic::abort_unwind #441

Closed
Closed
@CAD97

Description

@CAD97

Proposal

Problem statement

Various use cases exist that can't or don't want to support being unwound through. Today, there are multiple ways of coverting an in-progress unwind into an abort, each with subtly different user experience when the abort occurs. By providing a function abort_unwind as part of core, we can standardize the experience.

Motivating examples or use cases

Perhaps the most notorious case of unwind-abort is the take_mut/replace_with pattern. The take_mut crate uses catch_unwind to respond to unwinds. The replace_with crate uses drop flag conditional drop glue to respond to unwinds. Both crates simply call abort on an unwind and don't show a more specific reason for what caused the abort.

Also highly notable is the soundness footgun that dropping the panic payload returned from catch_unwind can itself cause an unwind. std necessarily uses catch_unwind1 and responds to an unwind with rtabort!.

I myself have used impl Drop types to cause a deliberate double panic (and thus an abort) in code that cannot soundly repair state when it gets unwound. This strategy generally creates a more useful crash message, as the runtime can see that the unwind will not be completed.

Solution sketch

The newly stabilized abort on extern "C" edges (“panic in a function that cannot unwind”) is the exact handling that we would like to have for all code regions that cannot be unwound. We also have catch_unwind already establishing the API shape of establishing an unwind handler.

/// Invokes a closure, aborting if the closure unwinds.
///
/// When compiled with aborting panics, this function is effectively a no-op.
/// With unwinding panics, an unwind results in another call into the panic
/// hook followed by a process abort.
///
/// # Notes
///
/// Instead of using this function, code should attempt to support unwinding.
/// Implementing [`Drop`] allows you to restore invariants uniformly in both
/// return and unwind paths.
///
/// If an unwind can lead to logical issues but not soundness issues, you
/// should allow the unwind. Opting out of [`UnwindSafe`] indicates to your
/// consumers that they need to consider correctness in the face of unwinds.
///
/// If an unwind would be unsound, then this function should be used in order
/// to prevent unwinds. However, note that `extern "C" fn` will automatically
/// convert unwinds to aborts, so using this function isn't necessary for FFI.
pub fn abort_unwind<F: FnOnce() -> R, R>(f: F) -> R {
    // extern "C" converts unwinding edges to unwind abort(abi)
    extern "C" fn abort_unwind_inner<F: FnOnce() -> R, R>(f: F) -> R {
        f()
    }

    abort_unwind_inner(f)
}

Alternatives

We can always do nothing; existing solutions in the ecosystem are sufficient. However, the way that this is usually done results in suboptimal diagnostics on an abort, and the required user code required to be sound is “obvious” enough that pulling in a 3rd party crate is unlikely.

I've chosen to use an inner function with extern "C" such that the public API signature doesn't show this implementation choice. It would be a valid choice to skip this step, and/or to use the #[rustc_nounwind] attribute instead of ABI.

Similarly, the language could provide a public #[nounwind] attribute to prevent unwinding instead of abort_unwind being library API. I argue that providing the library API is the better choice since the added friction discouraging the use of nounwind shims is a good thing overall.

Links and related work

I have participated in wg-unwind, but this is my own individual idea.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

Footnotes

  1. There needs to be at least one catch frame on the stack to ensure that raising an unwind can actually unwind the stack.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ACP-acceptedAPI Change Proposal is accepted (seconded with no objections)T-libs-apiapi-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions