From 1564fae01152946f58e67658c0b5d884fedaecdc Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 10 Aug 2018 11:06:30 +0200 Subject: [PATCH] Allow setting the panic hook with parity-clib (#9292) * Allow setting the panic hook with parity-clib * Make all FFI functions unsafe * Fix comment * Fix concern --- Cargo.lock | 1 + parity-clib/Cargo.toml | 1 + parity-clib/parity.h | 17 +++ parity-clib/src/lib.rs | 245 +++++++++++++++++++------------------ parity/lib.rs | 1 - parity/main.rs | 3 +- util/panic_hook/src/lib.rs | 43 ++++--- 7 files changed, 168 insertions(+), 143 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f529f2bc6..a933343a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1947,6 +1947,7 @@ source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c9 name = "parity-clib" version = "1.12.0" dependencies = [ + "panic_hook 0.1.0", "parity-ethereum 2.1.0", ] diff --git a/parity-clib/Cargo.toml b/parity-clib/Cargo.toml index 3a1e95b5f..32ddf0ecd 100644 --- a/parity-clib/Cargo.toml +++ b/parity-clib/Cargo.toml @@ -10,6 +10,7 @@ name = "parity" crate-type = ["cdylib", "staticlib"] [dependencies] +panic_hook = { path = "../util/panic_hook" } parity-ethereum = { path = "../", default-features = false } [features] diff --git a/parity-clib/parity.h b/parity-clib/parity.h index f647395ce..9be077b4d 100644 --- a/parity-clib/parity.h +++ b/parity-clib/parity.h @@ -102,6 +102,23 @@ void parity_destroy(void* parity); /// int parity_rpc(void* parity, const char* rpc, size_t len, char* out_str, size_t* out_len); +/// Sets a callback to call when a panic happens in the Rust code. +/// +/// The callback takes as parameter the custom param (the one passed to this function), plus the +/// panic message. You are expected to log the panic message somehow, in order to communicate it to +/// the user. A panic always indicates a bug in Parity. +/// +/// Note that this method sets the panic hook for the whole program, and not just for Parity. In +/// other words, if you use multiple Rust libraries at once (and not just Parity), then a panic +/// in any Rust code will call this callback as well. +/// +/// ## Thread safety +/// +/// The callback can be called from any thread and multiple times simultaneously. Make sure that +/// your code is thread safe. +/// +int parity_set_panic_hook(void (*cb)(void* param, const char* msg, size_t msg_len), void* param); + #ifdef __cplusplus } #endif diff --git a/parity-clib/src/lib.rs b/parity-clib/src/lib.rs index 563eafd73..ad0c8a032 100644 --- a/parity-clib/src/lib.rs +++ b/parity-clib/src/lib.rs @@ -18,6 +18,7 @@ //! duplicating documentation. extern crate parity_ethereum; +extern crate panic_hook; use std::os::raw::{c_char, c_void, c_int}; use std::panic; @@ -33,132 +34,132 @@ pub struct ParityParams { } #[no_mangle] -pub extern fn parity_config_from_cli(args: *const *const c_char, args_lens: *const usize, len: usize, output: *mut *mut c_void) -> c_int { - unsafe { - panic::catch_unwind(|| { - *output = ptr::null_mut(); +pub unsafe extern fn parity_config_from_cli(args: *const *const c_char, args_lens: *const usize, len: usize, output: *mut *mut c_void) -> c_int { + panic::catch_unwind(|| { + *output = ptr::null_mut(); - let args = { - let arg_ptrs = slice::from_raw_parts(args, len); - let arg_lens = slice::from_raw_parts(args_lens, len); + let args = { + let arg_ptrs = slice::from_raw_parts(args, len); + let arg_lens = slice::from_raw_parts(args_lens, len); - let mut args = Vec::with_capacity(len + 1); - args.push("parity".to_owned()); + let mut args = Vec::with_capacity(len + 1); + args.push("parity".to_owned()); - for (&arg, &len) in arg_ptrs.iter().zip(arg_lens.iter()) { - let string = slice::from_raw_parts(arg as *const u8, len); - match String::from_utf8(string.to_owned()) { - Ok(a) => args.push(a), - Err(_) => return 1, - }; - } - - args - }; - - match parity_ethereum::Configuration::parse_cli(&args) { - Ok(mut cfg) => { - // Always disable the auto-updater when used as a library. - cfg.args.arg_auto_update = "none".to_owned(); - - let cfg = Box::into_raw(Box::new(cfg)); - *output = cfg as *mut _; - 0 - }, - Err(_) => { - 1 - }, - } - }).unwrap_or(1) - } -} - -#[no_mangle] -pub extern fn parity_config_destroy(cfg: *mut c_void) { - unsafe { - let _ = panic::catch_unwind(|| { - let _cfg = Box::from_raw(cfg as *mut parity_ethereum::Configuration); - }); - } -} - -#[no_mangle] -pub extern fn parity_start(cfg: *const ParityParams, output: *mut *mut c_void) -> c_int { - unsafe { - panic::catch_unwind(|| { - *output = ptr::null_mut(); - let cfg: &ParityParams = &*cfg; - - let config = Box::from_raw(cfg.configuration as *mut parity_ethereum::Configuration); - - let on_client_restart_cb = { - struct Cb(Option, *mut c_void); - unsafe impl Send for Cb {} - unsafe impl Sync for Cb {} - impl Cb { - fn call(&self, new_chain: String) { - if let Some(ref cb) = self.0 { - cb(self.1, new_chain.as_bytes().as_ptr() as *const _, new_chain.len()) - } - } - } - let cb = Cb(cfg.on_client_restart_cb, cfg.on_client_restart_cb_custom); - move |new_chain: String| { cb.call(new_chain); } - }; - - let action = match parity_ethereum::start(*config, on_client_restart_cb, || {}) { - Ok(action) => action, - Err(_) => return 1, - }; - - match action { - parity_ethereum::ExecutionAction::Instant(Some(s)) => { println!("{}", s); 0 }, - parity_ethereum::ExecutionAction::Instant(None) => 0, - parity_ethereum::ExecutionAction::Running(client) => { - *output = Box::into_raw(Box::::new(client)) as *mut c_void; - 0 - } - } - }).unwrap_or(1) - } -} - -#[no_mangle] -pub extern fn parity_destroy(client: *mut c_void) { - unsafe { - let _ = panic::catch_unwind(|| { - let client = Box::from_raw(client as *mut parity_ethereum::RunningClient); - client.shutdown(); - }); - } -} - -#[no_mangle] -pub extern fn parity_rpc(client: *mut c_void, query: *const char, len: usize, out_str: *mut c_char, out_len: *mut usize) -> c_int { - unsafe { - panic::catch_unwind(|| { - let client: &mut parity_ethereum::RunningClient = &mut *(client as *mut parity_ethereum::RunningClient); - - let query_str = { - let string = slice::from_raw_parts(query as *const u8, len); - match str::from_utf8(string) { - Ok(a) => a, + for (&arg, &len) in arg_ptrs.iter().zip(arg_lens.iter()) { + let string = slice::from_raw_parts(arg as *const u8, len); + match String::from_utf8(string.to_owned()) { + Ok(a) => args.push(a), Err(_) => return 1, - } - }; - - if let Some(output) = client.rpc_query_sync(query_str) { - let q_out_len = output.as_bytes().len(); - if *out_len < q_out_len { - return 1; - } - - ptr::copy_nonoverlapping(output.as_bytes().as_ptr(), out_str as *mut u8, q_out_len); - *out_len = q_out_len; - 0 - } else { - 1 + }; } - }).unwrap_or(1) + + args + }; + + match parity_ethereum::Configuration::parse_cli(&args) { + Ok(mut cfg) => { + // Always disable the auto-updater when used as a library. + cfg.args.arg_auto_update = "none".to_owned(); + + let cfg = Box::into_raw(Box::new(cfg)); + *output = cfg as *mut _; + 0 + }, + Err(_) => { + 1 + }, + } + }).unwrap_or(1) +} + +#[no_mangle] +pub unsafe extern fn parity_config_destroy(cfg: *mut c_void) { + let _ = panic::catch_unwind(|| { + let _cfg = Box::from_raw(cfg as *mut parity_ethereum::Configuration); + }); +} + +#[no_mangle] +pub unsafe extern fn parity_start(cfg: *const ParityParams, output: *mut *mut c_void) -> c_int { + panic::catch_unwind(|| { + *output = ptr::null_mut(); + let cfg: &ParityParams = &*cfg; + + let config = Box::from_raw(cfg.configuration as *mut parity_ethereum::Configuration); + + let on_client_restart_cb = { + let cb = CallbackStr(cfg.on_client_restart_cb, cfg.on_client_restart_cb_custom); + move |new_chain: String| { cb.call(&new_chain); } + }; + + let action = match parity_ethereum::start(*config, on_client_restart_cb, || {}) { + Ok(action) => action, + Err(_) => return 1, + }; + + match action { + parity_ethereum::ExecutionAction::Instant(Some(s)) => { println!("{}", s); 0 }, + parity_ethereum::ExecutionAction::Instant(None) => 0, + parity_ethereum::ExecutionAction::Running(client) => { + *output = Box::into_raw(Box::::new(client)) as *mut c_void; + 0 + } + } + }).unwrap_or(1) +} + +#[no_mangle] +pub unsafe extern fn parity_destroy(client: *mut c_void) { + let _ = panic::catch_unwind(|| { + let client = Box::from_raw(client as *mut parity_ethereum::RunningClient); + client.shutdown(); + }); +} + +#[no_mangle] +pub unsafe extern fn parity_rpc(client: *mut c_void, query: *const char, len: usize, out_str: *mut c_char, out_len: *mut usize) -> c_int { + panic::catch_unwind(|| { + let client: &mut parity_ethereum::RunningClient = &mut *(client as *mut parity_ethereum::RunningClient); + + let query_str = { + let string = slice::from_raw_parts(query as *const u8, len); + match str::from_utf8(string) { + Ok(a) => a, + Err(_) => return 1, + } + }; + + if let Some(output) = client.rpc_query_sync(query_str) { + let q_out_len = output.as_bytes().len(); + if *out_len < q_out_len { + return 1; + } + + ptr::copy_nonoverlapping(output.as_bytes().as_ptr(), out_str as *mut u8, q_out_len); + *out_len = q_out_len; + 0 + } else { + 1 + } + }).unwrap_or(1) +} + +#[no_mangle] +pub unsafe extern fn parity_set_panic_hook(callback: extern "C" fn(*mut c_void, *const c_char, usize), param: *mut c_void) { + let cb = CallbackStr(Some(callback), param); + panic_hook::set_with(move |panic_msg| { + cb.call(panic_msg); + }); +} + +// Internal structure for handling callbacks that get passed a string. +struct CallbackStr(Option, *mut c_void); +unsafe impl Send for CallbackStr {} +unsafe impl Sync for CallbackStr {} +impl CallbackStr { + fn call(&self, new_chain: &str) { + if let Some(ref cb) = self.0 { + cb(self.1, new_chain.as_bytes().as_ptr() as *const _, new_chain.len()) + } } } diff --git a/parity/lib.rs b/parity/lib.rs index 84cacf07e..a2ea11ffe 100644 --- a/parity/lib.rs +++ b/parity/lib.rs @@ -57,7 +57,6 @@ extern crate ethcore_transaction as transaction; extern crate ethereum_types; extern crate ethkey; extern crate kvdb; -extern crate panic_hook; extern crate parity_hash_fetch as hash_fetch; extern crate parity_ipfs_api; extern crate parity_local_store as local_store; diff --git a/parity/main.rs b/parity/main.rs index 9256373ca..1254c3472 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -253,7 +253,8 @@ fn main_direct(force_can_restart: bool) -> i32 { panic_hook::set_with({ let e = exit.clone(); let exiting = exiting.clone(); - move || { + move |panic_msg| { + let _ = stdio::stderr().write_all(panic_msg.as_bytes()); if !exiting.swap(true, Ordering::SeqCst) { *e.0.lock() = ExitStatus { panicking: true, diff --git a/util/panic_hook/src/lib.rs b/util/panic_hook/src/lib.rs index cc7ed7ded..0c8552509 100644 --- a/util/panic_hook/src/lib.rs +++ b/util/panic_hook/src/lib.rs @@ -24,16 +24,26 @@ use std::thread; use std::process; use backtrace::Backtrace; -/// Set the panic hook +/// Set the panic hook to write to stderr and abort the process when a panic happens. pub fn set_abort() { - set_with(|| process::abort()); + set_with(|msg| { + let _ = io::stderr().write_all(msg.as_bytes()); + process::abort() + }); } -/// Set the panic hook with a closure to be called afterwards. -pub fn set_with(f: F) { +/// Set the panic hook with a closure to be called. The closure receives the panic message. +/// +/// Depending on how Parity was compiled, after the closure has been executed, either the process +/// aborts or unwinding starts. +/// +/// If you panic within the closure, a double panic happens and the process will stop. +pub fn set_with(f: F) +where F: Fn(&str) + Send + Sync + 'static +{ panic::set_hook(Box::new(move |info| { - panic_hook(info); - f(); + let msg = gen_panic_msg(info); + f(&msg); })); } @@ -43,7 +53,7 @@ This is a bug. Please report it at: https://github.com/paritytech/parity-ethereum/issues/new "; -fn panic_hook(info: &PanicInfo) { +fn gen_panic_msg(info: &PanicInfo) -> String { let location = info.location(); let file = location.as_ref().map(|l| l.file()).unwrap_or(""); let line = location.as_ref().map(|l| l.line()).unwrap_or(0); @@ -61,18 +71,13 @@ fn panic_hook(info: &PanicInfo) { let backtrace = Backtrace::new(); - let mut stderr = io::stderr(); + format!(r#" - let _ = writeln!(stderr, ""); - let _ = writeln!(stderr, "===================="); - let _ = writeln!(stderr, ""); - let _ = writeln!(stderr, "{:?}", backtrace); - let _ = writeln!(stderr, ""); - let _ = writeln!( - stderr, - "Thread '{}' panicked at '{}', {}:{}", - name, msg, file, line - ); +==================== - let _ = writeln!(stderr, "{}", ABOUT_PANIC); +{backtrace:?} + +Thread '{name}' panicked at '{msg}', {file}:{line} +{about} +"#, backtrace = backtrace, name = name, msg = msg, file = file, line = line, about = ABOUT_PANIC) }