diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index 43e7365e80c..63614f9d047 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -5,21 +5,21 @@ # Summary -Stabilize `std::thread::catch_panic` after removing the `Send` and `'static` -bounds from the closure parameter. +Move `std::thread::catch_panic` to `std::panic::recover` after removing the +`Send` bound from the closure parameter. # Motivation In today's stable Rust it's not possible to catch a panic on the thread that -caused it. There are a number of situations, however, where catching a panic is +caused it. There are a number of situations, however, where this is either required for correctness or necessary for building a useful abstraction: * It is currently defined as undefined behavior to have a Rust program panic across an FFI boundary. For example if C calls into Rust and Rust panics, then this is undefined behavior. Being able to catch a panic will allow writing - C apis in Rust that do not risk aborting the process they are embedded into. + C APIs in Rust that do not risk aborting the process they are embedded into. -* Abstactions like thread pools want to catch the panics of tasks being run +* Abstractions like thread pools want to catch the panics of tasks being run instead of having the thread torn down (and having to spawn a new thread). Stabilizing the `catch_panic` function would enable these two use cases, but @@ -34,8 +34,10 @@ This function will run the closure `f` and if it panics return `Err(Box)`. If the closure doesn't panic it will return `Ok(val)` where `val` is the returned value of the closure. The closure, however, is restricted to only close over `Send` and `'static` data. These bounds can be overly restrictive, and due -to thread-local storage they can be subverted, making it unclear what purpose -they serve. This RFC proposes to remove the bounds as well. +to thread-local storage [they can be subverted][tls-subvert], making it unclear +what purpose they serve. This RFC proposes to remove the bounds as well. + +[tls-subvert]: https://github.com/rust-lang/rust/issues/25662 Historically Rust has purposefully avoided the foray into the situation of catching panics, largely because of a problem typically referred to as @@ -71,7 +73,7 @@ in the face of exceptions, but languages often have constructs to enable these sorts of witnesses. Two primary methods of doing so are something akin to finally blocks (code run on a normal or exceptional return) or just catching the exception. In both cases code which later runs that has access to the original -data structure then it will see the broken invariants. +data structure will see the broken invariants. Now that we've got a better understanding of how an exception might cause a bug (e.g. how code can be "exception unsafe"), let's take a look how we can make @@ -176,12 +178,13 @@ this RFC. # Detailed design -At its heart, the change this RFC is proposing is to stabilize -`std::thread::catch_panic` after removing the `Send` and `'static` bounds from -the closure parameter, modifying the signature to be: +At its heart, the change this RFC is proposing is to move +`std::thread::catch_panic` to a new `std::panic` module and rename the function +to `catch`. Additionally, the `Send` bound from the closure parameter will be +removed (`'static` will stay), modifying the signature to be: ```rust -fn catch_panic R, R>(f: F) -> thread::Result +fn recover R + 'static, R>(f: F) -> thread::Result ``` More generally, however, this RFC also claims that this stable function does @@ -194,29 +197,29 @@ exist in the form of panics. What this RFC is adding, however, is a construct via which to catch these exceptions within a thread, bringing the standard library closer to the exception support in other languages. -Catching a panic (and especially not having `'static` on the bounds list) makes -it easier to observe broken invariants of data structures shared across the -`catch_panic` boundary, which can possibly increase the likelihood of exception -safety issues arising. +Catching a panic makes it easier to observe broken invariants of data structures +shared across the `catch_panic` boundary, which can possibly increase the +likelihood of exception safety issues arising. The risk of this step is that catching panics becomes an idiomatic way to deal with error-handling, thereby making exception safety much more of a headache -than it is today. Whereas we intend for the `catch_panic` function to only be -used where it's absolutely necessary, e.g. for FFI boundaries. How do we ensure -that `catch_panic` isn't overused? +than it is today (as it's more likely that a broken invariant is later +witnessed). The `catch_panic` function is intended to only be used +where it's absolutely necessary, e.g. for FFI boundaries, but how can it be +ensured that `catch_panic` isn't overused? -There are two key reasons we don't except `catch_panic` to become idiomatic: +There are two key reasons `catch_panic` likely won't become idiomatic: -1. We have already established very strong conventions around error handling, - and in particular around the use of panic and `Result`, and stabilized usage - around them in the standard library. There is little chance these conventions +1. There are already strong and established conventions around error handling, + and in particular around the use of panic and `Result` with stabilized usage + of them in the standard library. There is little chance these conventions would change overnight. -2. We have long intended to provide an option to treat every use of `panic!` as - an abort, which is motivated by portability, compile time, binary size, and a - number of other factors. Assuming we take this step, it would be extremely - unwise for a library to signal expected errors via panics and rely on - consumers using `catch_panic` to handle them. +2. There has long been a desire to treat every use of `panic!` as an abort + which is motivated by portability, compile time, binary size, and a number of + other factors. Assuming this step is taken, it would be extremely unwise for + a library to signal expected errors via panics and rely on consumers using + `catch_panic` to handle them. For reference, here's a summary of the conventions around `Result` and `panic`, which still hold good after this RFC: @@ -273,16 +276,24 @@ roughly analogous to an opaque "an unexpected error has occurred" message. Stabilizing `catch_panic` does little to change the tradeoffs around `Result` and `panic` that led to these conventions. -## Why remove the bounds? +## Why remove `Send`? + +One of the primary use cases of `recover` is in an FFI context, where lots +of `*mut` and `*const` pointers are flying around. These two types aren't +`Send` by default, so having their values cross the `catch_panic` boundary +would be highly un-ergonomic (albeit still possible). As a result, this RFC +proposes removing the `Send` bound from the function. + +## Why keep `'static`? + +This RFC proposes leaving the `'static` bound on the closure parameter for now. +There isn't a clearly strong case (such as for `Send`) to remove this parameter +just yet, and it helps mitigate exception safety issues related to shared +references across the `recover` boundary. -The main reason to remove the `'static` and `Send` bounds on `catch_panic` is -that they don't actually enforce anything. Using thread-local storage, it's -possible to share mutable data across a call to `catch_panic` even if that data -isn't `'static` or `Send`. And allowing borrowed data, in particular, is helpful -for thread pools that need to execute closures with borrowed data within them; -essentially, the worker threads are executing multiple "semantic threads" over -their lifetime, and the `catch_panic` boundary represents the end of these -"semantic threads". +There is conversely also not a clearly strong case for *keeping* this bound, but +as it's the more conservative route (and backwards compatible to remove) it will +remain for now. # Drawbacks