From cbfa61282fc34204b9b66554e06189c2d0b4d085 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Tue, 22 Sep 2015 08:40:15 +0200 Subject: [PATCH 1/2] Simplify on_panic callback handling The registration of `panicking::on_panic` as a general-purpose callback is overcomplicated and can fail. Instead, invoking it explicitly removes the need for locking and paves the way for further improvements. --- src/libstd/sys/common/unwind/mod.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/libstd/sys/common/unwind/mod.rs b/src/libstd/sys/common/unwind/mod.rs index 738681c3cfe..47f6bd9315d 100644 --- a/src/libstd/sys/common/unwind/mod.rs +++ b/src/libstd/sys/common/unwind/mod.rs @@ -244,22 +244,12 @@ pub fn begin_unwind(msg: M, file_line: &(&'static str, u32)) -> ! #[inline(never)] #[cold] // this is the slow path, please never inline this fn begin_unwind_inner(msg: Box, file_line: &(&'static str, u32)) -> ! { - // Make sure the default failure handler is registered before we look at the - // callbacks. We also use a raw sys-based mutex here instead of a - // `std::sync` one as accessing TLS can cause weird recursive problems (and - // we don't need poison checking). - unsafe { - static LOCK: Mutex = Mutex::new(); - static mut INIT: bool = false; - LOCK.lock(); - if !INIT { - register(panicking::on_panic); - INIT = true; - } - LOCK.unlock(); - } + let (file, line) = *file_line; - // First, invoke call the user-defined callbacks triggered on thread panic. + // First, invoke the default panic handler. + panicking::on_panic(&*msg, file, line); + + // Then, invoke call the user-defined callbacks triggered on thread panic. // // By the time that we see a callback has been registered (by reading // MAX_CALLBACKS), the actual callback itself may have not been stored yet, @@ -275,7 +265,6 @@ fn begin_unwind_inner(msg: Box, 0 => {} n => { let f: Callback = unsafe { mem::transmute(n) }; - let (file, line) = *file_line; f(&*msg, file, line); } } From c6d277ade642a8cf166d6e4706add90fb9181ec2 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Tue, 22 Sep 2015 18:51:04 +0200 Subject: [PATCH 2/2] Remove unwind::register The `register` function is unstable and it is not used anymore, hence it can be removed (together with the now-unused `Callback` type and `static` variables). --- src/libstd/sys/common/unwind/mod.rs | 71 +---------------------------- 1 file changed, 2 insertions(+), 69 deletions(-) diff --git a/src/libstd/sys/common/unwind/mod.rs b/src/libstd/sys/common/unwind/mod.rs index 47f6bd9315d..af6c1184504 100644 --- a/src/libstd/sys/common/unwind/mod.rs +++ b/src/libstd/sys/common/unwind/mod.rs @@ -92,23 +92,6 @@ pub mod imp; #[path = "gcc.rs"] #[doc(hidden)] pub mod imp; -pub type Callback = fn(msg: &(Any + Send), file: &'static str, line: u32); - -// Variables used for invoking callbacks when a thread starts to unwind. -// -// For more information, see below. -const MAX_CALLBACKS: usize = 16; -static CALLBACKS: [atomic::AtomicUsize; MAX_CALLBACKS] = - [atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0), - atomic::AtomicUsize::new(0), atomic::AtomicUsize::new(0)]; -static CALLBACK_CNT: atomic::AtomicUsize = atomic::AtomicUsize::new(0); - thread_local! { static PANICKING: Cell = Cell::new(false) } /// Invoke a closure, capturing the cause of panic if one occurs. @@ -249,29 +232,6 @@ fn begin_unwind_inner(msg: Box, // First, invoke the default panic handler. panicking::on_panic(&*msg, file, line); - // Then, invoke call the user-defined callbacks triggered on thread panic. - // - // By the time that we see a callback has been registered (by reading - // MAX_CALLBACKS), the actual callback itself may have not been stored yet, - // so we just chalk it up to a race condition and move on to the next - // callback. Additionally, CALLBACK_CNT may briefly be higher than - // MAX_CALLBACKS, so we're sure to clamp it as necessary. - let callbacks = { - let amt = CALLBACK_CNT.load(Ordering::SeqCst); - &CALLBACKS[..cmp::min(amt, MAX_CALLBACKS)] - }; - for cb in callbacks { - match cb.load(Ordering::SeqCst) { - 0 => {} - n => { - let f: Callback = unsafe { mem::transmute(n) }; - f(&*msg, file, line); - } - } - }; - - // Now that we've run all the necessary unwind callbacks, we actually - // perform the unwinding. if panicking() { // If a thread panics while it's already unwinding then we // have limited options. Currently our preference is to @@ -282,34 +242,7 @@ fn begin_unwind_inner(msg: Box, unsafe { intrinsics::abort() } } PANICKING.with(|s| s.set(true)); + + // Finally, perform the unwinding. rust_panic(msg); } - -/// Register a callback to be invoked when a thread unwinds. -/// -/// This is an unsafe and experimental API which allows for an arbitrary -/// callback to be invoked when a thread panics. This callback is invoked on both -/// the initial unwinding and a double unwinding if one occurs. Additionally, -/// the local `Thread` will be in place for the duration of the callback, and -/// the callback must ensure that it remains in place once the callback returns. -/// -/// Only a limited number of callbacks can be registered, and this function -/// returns whether the callback was successfully registered or not. It is not -/// currently possible to unregister a callback once it has been registered. -pub unsafe fn register(f: Callback) -> bool { - match CALLBACK_CNT.fetch_add(1, Ordering::SeqCst) { - // The invocation code has knowledge of this window where the count has - // been incremented, but the callback has not been stored. We're - // guaranteed that the slot we're storing into is 0. - n if n < MAX_CALLBACKS => { - let prev = CALLBACKS[n].swap(mem::transmute(f), Ordering::SeqCst); - rtassert!(prev == 0); - true - } - // If we accidentally bumped the count too high, pull it back. - _ => { - CALLBACK_CNT.store(MAX_CALLBACKS, Ordering::SeqCst); - false - } - } -}