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.

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

RFC: Abort by default #1759

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
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
74 changes: 74 additions & 0 deletions text/0000-abort-by-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
- Feature Name: abort_by_default
- Start Date: 2016-10-01
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Specify `panic=abort` in `Cargo.toml` when the user does `cargo new --bin`.

# Motivation
[motivation]: #motivation

## Performance

Generally, the performance of bigger programs [improves by 10%](https://www.youtube.com/watch?v=mRGb4hoGuPs), which is no small amount. When overflow checking is enabled, it is as high as 2x, making overflowing checking plausible in production.

The performance gains mainly originates in the fact that it is a lot easier for the compiler to optimize, due to the unwind path disappearing, and hence reasoning about the code becomes easier.

## Binary size

Binary size improves by 10% as well. This is due to the lack of landing pads which are injected in the stack when unwinding is enabled.

## Compile time

Compile time in debug mode improves by 20% on average. In release mode, the number is around 10%.

## Runtime size

Unwinding requires a fair amount of runtime code, which can be seen as conflicting with the goals of Rust.

## Correctness

You often see people abusing `std::panic::catch_unwind` for exception handling. Forcing the user to explicitly opt in to unwinding will make it harder to unintentionally misuse it.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think I have ever actually seen this in practice. Where does this pop up often?

Copy link

@codyps codyps Oct 2, 2016

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On /r/rust people regularly talk about how "nice it is to have exceptions".

Copy link

@ArtemGr ArtemGr Oct 5, 2016

Choose a reason for hiding this comment

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

"Forcing the user to explicitly opt in to unwinding will make it harder to unintentionally misuse it."

At least one person has done this

It was an intentional misuse though. Cf. https://www.reddit.com/r/rust/comments/53ft4m/my_experience_rewriting_enjarify_in_rust/d7ss1vy


# Detailed design
[design]: #detailed-design

Have `cargo` generate `panic=abort` to the `Cargo.toml`. Whenever the user inteacts with `cargo` (by using a command) and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user:

Warning: No panic strategy was specified, added unwinding to the `Cargo.toml` file, please consider change it to your needs.

For libraries, nothing is modified, as they inherit the strategy of the binary.

After several release cycles, an extension could be added, which makes specifying the strategy mandatory.

# Drawbacks
[drawbacks]: #drawbacks

It makes panics global by default (i.e., panicking in some thread will abort the whole program).
Copy link
Member

Choose a reason for hiding this comment

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

This will break ThreadPool::panic_count.

Copy link

Choose a reason for hiding this comment

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

Well, not really: with panic=abort one will never see a non-zero panic_count, which is still functional behavior as long as the program's functionality doesn't rely on panics being non-fatal to the program.

Copy link
Member

Choose a reason for hiding this comment

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

which is still functional behavior

If 'abort by default' was implemented, programs that use panic_count now no longer function in the same way. The only way in which it's "functional" is that there is no compile-time error, which seems bad.

Copy link

@codyps codyps Oct 4, 2016

Choose a reason for hiding this comment

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

panic_count is really beside the point here, the actual behavior that would change is that a panic in a thread would abort the entire program instead of being caught by the thread pool. Which seems expected (given the RFC). The objection here seems to just be "panics won't be unwinds anymore". Which is true.


It might make it tempting to ignore the existence of an unwind option and consequently shaping the code after panicking being aborting.

## Unwinding is not bad per se

Unwinding has numerous advantages. Especially for certain classes of applications. These includes better error handling and better cleanup.

Unwinding is especially important to long-running applications.

# Alternatives
[alternatives]: #alternatives

Keep unwinding as default.

Make Cargo set `panic=abort` in all new binaries.

Use unwind in `debug` and abort in `release`.

Make use of more exotic instructions like `bound`, allowing e.g. overflow or bound checking without branches. This comes at the expense of error output.

# Unresolved questions
[unresolved]: #unresolved-questions

None.