From 0a13f1abafe70cddf34bf2b2ba3946c418ed6241 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 31 Aug 2015 08:51:53 -0700 Subject: [PATCH] std: Rename thread::catch_panic to panic::recover This commit is an implementation of [RFC 1236] and [RFC 1323] which rename the `thread::catch_panic` function to `panic::recover` while also replacing the `Send + 'static` bounds with a new `PanicSafe` bound. [RFC 1236]: https://github.com/rust-lang/rfcs/pull/1236 [RFC 1323]: https://github.com/rust-lang/rfcs/pull/1323 cc #27719 --- src/libstd/lib.rs | 3 + src/libstd/panic.rs | 255 +++++++++++++++++++ src/libstd/rt.rs | 5 +- src/libstd/thread/mod.rs | 19 +- src/test/compile-fail/not-panic-safe-2.rs | 24 ++ src/test/compile-fail/not-panic-safe-3.rs | 23 ++ src/test/compile-fail/not-panic-safe-4.rs | 22 ++ src/test/compile-fail/not-panic-safe-5.rs | 21 ++ src/test/compile-fail/not-panic-safe-6.rs | 23 ++ src/test/compile-fail/not-panic-safe.rs | 20 ++ src/test/run-pass/binary-heap-panic-safe.rs | 33 +-- src/test/run-pass/panic-safe.rs | 51 ++++ src/test/run-pass/running-with-no-runtime.rs | 8 +- 13 files changed, 462 insertions(+), 45 deletions(-) create mode 100644 src/libstd/panic.rs create mode 100644 src/test/compile-fail/not-panic-safe-2.rs create mode 100644 src/test/compile-fail/not-panic-safe-3.rs create mode 100644 src/test/compile-fail/not-panic-safe-4.rs create mode 100644 src/test/compile-fail/not-panic-safe-5.rs create mode 100644 src/test/compile-fail/not-panic-safe-6.rs create mode 100644 src/test/compile-fail/not-panic-safe.rs create mode 100644 src/test/run-pass/panic-safe.rs diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 0385dff65d1..8e6c32ff2fc 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -248,6 +248,7 @@ #![feature(link_args)] #![feature(linkage)] #![feature(macro_reexport)] +#![feature(on_unimplemented)] #![feature(oom)] #![feature(optin_builtin_traits)] #![feature(placement_in_syntax)] @@ -255,6 +256,7 @@ #![feature(range_inclusive)] #![feature(raw)] #![feature(reflect_marker)] +#![feature(shared)] #![feature(slice_bytes)] #![feature(slice_concat_ext)] #![feature(slice_patterns)] @@ -424,6 +426,7 @@ pub mod fs; pub mod io; pub mod net; pub mod os; +pub mod panic; pub mod path; pub mod process; pub mod sync; diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs new file mode 100644 index 00000000000..6e4ba337b08 --- /dev/null +++ b/src/libstd/panic.rs @@ -0,0 +1,255 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Panic support in the standard library + +#![unstable(feature = "std_panic", reason = "awaiting feedback", + issue = "27719")] + +use cell::UnsafeCell; +use ops::{Deref, DerefMut}; +use ptr::{Unique, Shared}; +use rc::Rc; +use sync::{Arc, Mutex, RwLock}; +use sys_common::unwind; +use thread::Result; + +/// A marker trait which represents "panic safe" types in Rust. +/// +/// This trait is implemented by default for many types and behaves similarly in +/// terms of inference of implementation to the `Send` and `Sync` traits. The +/// purpose of this trait is to encode what types are safe to cross a `recover` +/// boundary with no fear of panic safety. +/// +/// ## What is panic safety? +/// +/// In Rust a function can "return" early if it either panics or calls a +/// function which transitively panics. This sort of control flow is not always +/// anticipated, and has the possibility of causing subtle bugs through a +/// combination of two cricial components: +/// +/// 1. A data structure is in a temporarily invalid state when the thread +/// panics. +/// 2. This broken invariant is then later observed. +/// +/// Typically in Rust it is difficult to perform step (2) because catching a +/// panic involves either spawning a thread (which in turns makes it difficult +/// to later witness broken invariants) or using the `recover` function in this +/// module. Additionally, even if an invariant is witness, it typically isn't a +/// problem in Rust because there's no uninitialized values (like in C or C++). +/// +/// It is possible, however, for **logical** invariants to be broken in Rust, +/// which can end up causing behavioral bugs. Another key aspect of panic safety +/// in Rust is that in the absence of `unsafe` code, a panic cannot lead to +/// memory unsafety. +/// +/// That was a bit of a whirlwind tour of panic safety, but for more information +/// about panic safety and how it applies to Rust, see an [associated RFC][rfc]. +/// +/// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1236-stabilize-catch-panic.md +/// +/// ## What is `RecoverSafe`? +/// +/// Now that we've got an idea of what panic safety is in Rust, it's also +/// important to understand that this trait represents. As mentioned above, one +/// way to witness broken invariants is through the `recover` function in this +/// module as it allows catching a panic and then re-using the environment of +/// the closure. +/// +/// Simply but, a type `T` implements `RecoverSafe` if it cannot easily allow +/// witnessing a broken invariant through the use of `recover` (catching a +/// panic). This trait is a marker trait, so it is automatically implemented for +/// many types, and it is also structurally composed (e.g. a struct is recover +/// safe if all of its components are recover safe). +/// +/// Note, however, that this is not an unsafe trait, so there is not a succinct +/// contract that this trait is providing. Instead it is intended as more of a +/// "speed bump" to alert users of `recover` that broken invariants may be +/// witnessed and may need to be accounted for. +/// +/// ## Who implements `RecoverSafe`? +/// +/// Types such as `&mut T` and `&RefCell` are examples which are **not** +/// recover safe. The general idea is that any mutable state which can be shared +/// across `recover` is not recover safe by default. This is because it is very +/// easy to witness a broken invariant outside of `recover` as the data is +/// simply accesed as usual. +/// +/// Types like `&Mutex`, however, are recover safe because they implement +/// poisoning by default. They still allow witnessing a broken invariant, but +/// they already provide their own "speed bumps" to do so. +/// +/// ## When should `RecoverSafe` be used? +/// +/// Is not intended that most types or functions need to worry about this trait. +/// It is only used as a bound on the `recover` function and as mentioned above, +/// the lack of `unsafe` means it is mostly an advisory. The `AssertRecoverSafe` +/// wrapper struct in this module can be used to force this trait to be +/// implemented for any closed over variables passed to the `recover` function +/// (more on this below). +#[unstable(feature = "recover", reason = "awaiting feedback", issue = "27719")] +#[rustc_on_unimplemented = "the type {Self} may not be safely transferred \ + across a recover boundary"] +pub trait RecoverSafe {} + +/// A marker trait representing types which do not contain an `UnsafeCell` by +/// value internally. +/// +/// This is a "helper marker trait" used to provide impl blocks for the +/// `RecoverSafe` trait, for more information see that documentation. +#[unstable(feature = "recover", reason = "awaiting feedback", issue = "27719")] +#[rustc_on_unimplemented = "the type {Self} contains interior mutability \ + and a reference may not be safely transferrable \ + across a recover boundary"] +pub trait NoUnsafeCell {} + +/// A simple wrapper around a type to assert that it is panic safe. +/// +/// When using `recover` it may be the case that some of the closed over +/// variables are not panic safe. For example if `&mut T` is captured the +/// compiler will generate a warning indicating that it is not panic safe. It +/// may not be the case, however, that this is actually a problem due to the +/// specific usage of `recover` if panic safety is specifically taken into +/// account. This wrapper struct is useful for a quick and lightweight +/// annotation that a variable is indeed panic safe. +/// +/// # Examples +/// +/// ``` +/// #![feature(recover, std_panic)] +/// +/// use std::panic::{self, AssertRecoverSafe}; +/// +/// let mut variable = 4; +/// +/// // This code will not compile becuause the closure captures `&mut variable` +/// // which is not considered panic safe by default. +/// +/// // panic::recover(|| { +/// // variable += 3; +/// // }); +/// +/// // This, however, will compile due to the `AssertRecoverSafe` wrapper +/// let result = { +/// let mut wrapper = AssertRecoverSafe::new(&mut variable); +/// panic::recover(move || { +/// **wrapper += 3; +/// }) +/// }; +/// // ... +/// ``` +#[unstable(feature = "recover", reason = "awaiting feedback", issue = "27719")] +pub struct AssertRecoverSafe(T); + +// Implementations of the `RecoverSafe` trait: +// +// * By default everything is recover safe +// * pointers T contains mutability of some form are not recover safe +// * Unique, an owning pointer, lifts an implementation +// * Types like Mutex/RwLock which are explicilty poisoned are recover safe +// * Our custom AssertRecoverSafe wrapper is indeed recover safe +impl RecoverSafe for .. {} +impl<'a, T: ?Sized> !RecoverSafe for &'a mut T {} +impl<'a, T: NoUnsafeCell + ?Sized> RecoverSafe for &'a T {} +impl RecoverSafe for *const T {} +impl RecoverSafe for *mut T {} +impl RecoverSafe for Unique {} +impl RecoverSafe for Shared {} +impl RecoverSafe for Mutex {} +impl RecoverSafe for RwLock {} +impl RecoverSafe for AssertRecoverSafe {} + +// not covered via the Shared impl above b/c the inner contents use +// Cell/AtomicUsize, but the usage here is recover safe so we can lift the +// impl up one level to Arc/Rc itself +impl RecoverSafe for Rc {} +impl RecoverSafe for Arc {} + +// Pretty simple implementations for the `NoUnsafeCell` marker trait, basically +// just saying that this is a marker trait and `UnsafeCell` is the only thing +// which doesn't implement it (which then transitively applies to everything +// else. +impl NoUnsafeCell for .. {} +impl !NoUnsafeCell for UnsafeCell {} + +impl AssertRecoverSafe { + /// Creates a new `AssertRecoverSafe` wrapper around the provided type. + #[unstable(feature = "recover", reason = "awaiting feedback", issue = "27719")] + pub fn new(t: T) -> AssertRecoverSafe { + AssertRecoverSafe(t) + } +} + +impl Deref for AssertRecoverSafe { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} + +impl DerefMut for AssertRecoverSafe { + fn deref_mut(&mut self) -> &mut T { + &mut self.0 + } +} + +/// Invokes a closure, capturing the cause of panic if one occurs. +/// +/// This function will return `Ok` with the closure's result if the closure +/// does not panic, and will return `Err(cause)` if the closure panics. The +/// `cause` returned is the object with which panic was originally invoked. +/// +/// It is currently undefined behavior to unwind from Rust code into foreign +/// code, so this function is particularly useful when Rust is called from +/// another language (normally C). This can run arbitrary Rust code, capturing a +/// panic and allowing a graceful handling of the error. +/// +/// It is **not** recommended to use this function for a general try/catch +/// mechanism. The `Result` type is more appropriate to use for functions that +/// can fail on a regular basis. +/// +/// The closure provided is required to adhere to the `RecoverSafe` to ensure +/// that all captured variables are safe to cross this recover boundary. The +/// purpose of this bound is to encode the concept of [exception safety][rfc] in +/// the type system. Most usage of this function should not need to worry about +/// this bound as programs are naturally panic safe without `unsafe` code. If it +/// becomes a problem the associated `AssertRecoverSafe` wrapper type in this +/// module can be used to quickly assert that the usage here is indeed exception +/// safe. +/// +/// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1236-stabilize-catch-panic.md +/// +/// # Examples +/// +/// ``` +/// #![feature(recover, std_panic)] +/// +/// use std::panic; +/// +/// let result = panic::recover(|| { +/// println!("hello!"); +/// }); +/// assert!(result.is_ok()); +/// +/// let result = panic::recover(|| { +/// panic!("oh no!"); +/// }); +/// assert!(result.is_err()); +/// ``` +#[unstable(feature = "recover", reason = "awaiting feedback", issue = "27719")] +pub fn recover R + RecoverSafe, R>(f: F) -> Result { + let mut result = None; + unsafe { + let result = &mut result; + try!(unwind::try(move || *result = Some(f()))) + } + Ok(result.unwrap()) +} diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index 44961611b7b..63fb9b561ff 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -24,10 +24,11 @@ use borrow::ToOwned; use mem; +use panic; use sys; use sys_common::thread_info::{self, NewThread}; use sys_common; -use thread::{self, Thread}; +use thread::Thread; // Reexport some of our utilities which are expected by other crates. pub use sys_common::unwind::{begin_unwind, begin_unwind_fmt}; @@ -57,7 +58,7 @@ fn lang_start(main: *const u8, argc: isize, argv: *const *const u8) -> isize { sys_common::args::init(argc, argv); // Let's run some code! - let res = thread::catch_panic(mem::transmute::<_, fn()>(main)); + let res = panic::recover(mem::transmute::<_, fn()>(main)); sys_common::cleanup(); res.is_err() }; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index aef47aa5726..d4f04d5d94f 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -355,26 +355,9 @@ pub fn panicking() -> bool { /// with exception safety. Furthermore, a `Send` bound is also required, /// providing the same safety guarantees as `thread::spawn` (ensuring the /// closure is properly isolated from the parent). -/// -/// # Examples -/// -/// ``` -/// #![feature(catch_panic)] -/// -/// use std::thread; -/// -/// let result = thread::catch_panic(|| { -/// println!("hello!"); -/// }); -/// assert!(result.is_ok()); -/// -/// let result = thread::catch_panic(|| { -/// panic!("oh no!"); -/// }); -/// assert!(result.is_err()); -/// ``` #[unstable(feature = "catch_panic", reason = "recent API addition", issue = "27719")] +#[rustc_deprecated(since = "1.6.0", reason = "renamed to std::panic::recover")] pub fn catch_panic(f: F) -> Result where F: FnOnce() -> R + Send + 'static { diff --git a/src/test/compile-fail/not-panic-safe-2.rs b/src/test/compile-fail/not-panic-safe-2.rs new file mode 100644 index 00000000000..47a65505d8a --- /dev/null +++ b/src/test/compile-fail/not-panic-safe-2.rs @@ -0,0 +1,24 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::rc::Rc; +use std::cell::RefCell; + +fn assert() {} + +fn main() { + assert::>>(); //~ ERROR: is not implemented + //~^ ERROR: is not implemented +} + diff --git a/src/test/compile-fail/not-panic-safe-3.rs b/src/test/compile-fail/not-panic-safe-3.rs new file mode 100644 index 00000000000..a0c7865eeb0 --- /dev/null +++ b/src/test/compile-fail/not-panic-safe-3.rs @@ -0,0 +1,23 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::sync::Arc; +use std::cell::RefCell; + +fn assert() {} + +fn main() { + assert::>>(); //~ ERROR: is not implemented + //~^ ERROR: is not implemented +} diff --git a/src/test/compile-fail/not-panic-safe-4.rs b/src/test/compile-fail/not-panic-safe-4.rs new file mode 100644 index 00000000000..9e716131525 --- /dev/null +++ b/src/test/compile-fail/not-panic-safe-4.rs @@ -0,0 +1,22 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::cell::RefCell; + +fn assert() {} + +fn main() { + assert::<&RefCell>(); //~ ERROR: is not implemented + //~^ ERROR is not implemented +} diff --git a/src/test/compile-fail/not-panic-safe-5.rs b/src/test/compile-fail/not-panic-safe-5.rs new file mode 100644 index 00000000000..1fa76c21f85 --- /dev/null +++ b/src/test/compile-fail/not-panic-safe-5.rs @@ -0,0 +1,21 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::cell::UnsafeCell; + +fn assert() {} + +fn main() { + assert::<*const UnsafeCell>(); //~ ERROR: is not implemented +} diff --git a/src/test/compile-fail/not-panic-safe-6.rs b/src/test/compile-fail/not-panic-safe-6.rs new file mode 100644 index 00000000000..90c730d3758 --- /dev/null +++ b/src/test/compile-fail/not-panic-safe-6.rs @@ -0,0 +1,23 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::cell::RefCell; + +fn assert() {} + +fn main() { + assert::<*mut RefCell>(); //~ ERROR: is not implemented + //~^ ERROR is not implemented +} + diff --git a/src/test/compile-fail/not-panic-safe.rs b/src/test/compile-fail/not-panic-safe.rs new file mode 100644 index 00000000000..f06464c5b1a --- /dev/null +++ b/src/test/compile-fail/not-panic-safe.rs @@ -0,0 +1,20 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; + +fn assert() {} + +fn main() { + assert::<&mut i32>(); //~ ERROR: RecoverSafe` is not implemented +} diff --git a/src/test/run-pass/binary-heap-panic-safe.rs b/src/test/run-pass/binary-heap-panic-safe.rs index 8bdac6808d0..d85fd3a2b6b 100644 --- a/src/test/run-pass/binary-heap-panic-safe.rs +++ b/src/test/run-pass/binary-heap-panic-safe.rs @@ -8,15 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(std_misc, binary_heap_extras, catch_panic, rand, sync_poison)] +#![feature(recover, rand, std_panic)] use std::__rand::{thread_rng, Rng}; -use std::thread; +use std::panic::{self, AssertRecoverSafe}; use std::collections::BinaryHeap; use std::cmp; -use std::sync::Arc; -use std::sync::Mutex; use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; static DROP_COUNTER: AtomicUsize = ATOMIC_USIZE_INIT; @@ -66,31 +64,24 @@ fn test_integrity() { // heapify the sane items rng.shuffle(&mut panic_ords); - let heap = Arc::new(Mutex::new(BinaryHeap::from_vec(panic_ords))); + let mut heap = BinaryHeap::from(panic_ords); let inner_data; { - let heap_ref = heap.clone(); - - // push the panicking item to the heap and catch the panic - let thread_result = thread::catch_panic(move || { - heap.lock().unwrap().push(panic_item); - }); + let thread_result = { + let mut heap_ref = AssertRecoverSafe::new(&mut heap); + panic::recover(move || { + heap_ref.push(panic_item); + }) + }; assert!(thread_result.is_err()); // Assert no elements were dropped let drops = DROP_COUNTER.load(Ordering::SeqCst); - //assert!(drops == 0, "Must not drop items. drops={}", drops); - - { - // now fetch the binary heap's data vector - let mutex_guard = match heap_ref.lock() { - Ok(x) => x, - Err(poison) => poison.into_inner(), - }; - inner_data = mutex_guard.clone().into_vec(); - } + assert!(drops == 0, "Must not drop items. drops={}", drops); + inner_data = heap.clone().into_vec(); + drop(heap); } let drops = DROP_COUNTER.load(Ordering::SeqCst); assert_eq!(drops, DATASZ); diff --git a/src/test/run-pass/panic-safe.rs b/src/test/run-pass/panic-safe.rs new file mode 100644 index 00000000000..cd2457e8a52 --- /dev/null +++ b/src/test/run-pass/panic-safe.rs @@ -0,0 +1,51 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![feature(recover)] + +use std::panic::RecoverSafe; +use std::cell::RefCell; +use std::sync::{Mutex, RwLock, Arc}; +use std::rc::Rc; + +struct Foo { a: i32 } + +fn assert() {} + +fn main() { + assert::(); + assert::<&i32>(); + assert::<*mut i32>(); + assert::<*const i32>(); + assert::(); + assert::(); + assert::<&str>(); + assert::(); + assert::<&Foo>(); + assert::>(); + assert::(); + assert::>(); + assert::>(); + assert::>(); + assert::>(); + assert::>(); + assert::>(); + + fn bar() { + assert::>(); + assert::>(); + } + fn baz() { + assert::>(); + assert::>(); + assert::>(); + } +} diff --git a/src/test/run-pass/running-with-no-runtime.rs b/src/test/run-pass/running-with-no-runtime.rs index 2b2dcb6efb5..c67bc8c8368 100644 --- a/src/test/run-pass/running-with-no-runtime.rs +++ b/src/test/run-pass/running-with-no-runtime.rs @@ -8,11 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(catch_panic, start)] +#![feature(std_panic, recover, start)] use std::ffi::CStr; use std::process::{Command, Output}; -use std::thread; +use std::panic; use std::str; #[start] @@ -22,8 +22,8 @@ fn start(argc: isize, argv: *const *const u8) -> isize { match **argv.offset(1) as char { '1' => {} '2' => println!("foo"), - '3' => assert!(thread::catch_panic(|| {}).is_ok()), - '4' => assert!(thread::catch_panic(|| panic!()).is_err()), + '3' => assert!(panic::recover(|| {}).is_ok()), + '4' => assert!(panic::recover(|| panic!()).is_err()), '5' => assert!(Command::new("test").spawn().is_err()), _ => panic!() }