From 66ea017d3b167b3f747c9751640d0c3623ea363e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Aug 2015 09:56:14 -0700 Subject: [PATCH 1/5] Add link to issue about catch_panic + TLS --- text/0000-stabilize-catch-panic.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index 43e7365e80c..a4f7371ac55 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -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 From c68967f80b8cca85d27f745f626c41cb9fc66a4b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Aug 2015 10:09:13 -0700 Subject: [PATCH 2/5] Final adjustments --- text/0000-stabilize-catch-panic.md | 33 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index a4f7371ac55..f21ba09eecd 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -11,15 +11,15 @@ bounds 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 @@ -73,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 @@ -203,22 +203,23 @@ 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: From 3e400d96becbf74aa05e69764b374e28543121eb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Aug 2015 15:20:42 -0700 Subject: [PATCH 3/5] Expand on why Send/'static bounds are removed --- text/0000-stabilize-catch-panic.md | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index f21ba09eecd..95a229e4d4d 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -278,14 +278,24 @@ and `panic` that led to these conventions. ## Why remove the bounds? -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 are a few reasons to remove the `'static` and `Send` bounds on the +`catch_panic` function: + +* One of the primary use cases of `catch_panic` 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. + +* A reason to remove the `'static` bound is that it doesn't provide rock-solid + exception-safety mitigation. Using thread-local storage it's possible to + share mutable data across a call to `catch_panic` even if that data isn't + `'static`. + +* 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". # Drawbacks From e78f3e9cebc1497ea80b09dc20cb10d34248bdc3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 19 Aug 2015 17:06:17 -0700 Subject: [PATCH 4/5] Put catch_panic in a new std::panic module instead --- text/0000-stabilize-catch-panic.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index 95a229e4d4d..7be3ff68a76 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -5,8 +5,8 @@ # 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::catch` after removing the `Send` +and `'static` bounds from the closure parameter. # Motivation @@ -178,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` and `'static` bounds from the closure +parameter will be removed, modifying the signature to be: ```rust -fn catch_panic R, R>(f: F) -> thread::Result +fn catch R, R>(f: F) -> thread::Result ``` More generally, however, this RFC also claims that this stable function does From 9ebc36978bc6bba6ab52021989be384d418170ab Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 26 Aug 2015 17:25:38 -0700 Subject: [PATCH 5/5] Rename to recover, re-add 'static --- text/0000-stabilize-catch-panic.md | 47 ++++++++++++++---------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/text/0000-stabilize-catch-panic.md b/text/0000-stabilize-catch-panic.md index 7be3ff68a76..63614f9d047 100644 --- a/text/0000-stabilize-catch-panic.md +++ b/text/0000-stabilize-catch-panic.md @@ -5,8 +5,8 @@ # Summary -Move `std::thread::catch_panic` to `std::panic::catch` 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 @@ -180,11 +180,11 @@ this RFC. 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` and `'static` bounds from the closure -parameter will be removed, modifying the signature to be: +to `catch`. Additionally, the `Send` bound from the closure parameter will be +removed (`'static` will stay), modifying the signature to be: ```rust -fn catch 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 @@ -197,10 +197,9 @@ 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 @@ -277,26 +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`? -There are a few reasons to remove the `'static` and `Send` bounds on the -`catch_panic` function: +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. -* One of the primary use cases of `catch_panic` 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`? -* A reason to remove the `'static` bound is that it doesn't provide rock-solid - exception-safety mitigation. Using thread-local storage it's possible to - share mutable data across a call to `catch_panic` even if that data isn't - `'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. -* 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