From ed73f419afc8d68526b7cac7e25c57a4a805055c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 22 Sep 2020 00:34:06 +0200 Subject: [PATCH 1/3] Do not panic while panicking --- primitives/runtime/src/lib.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index eb8bbb38a6ffe..ece93568c900e 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -794,8 +794,13 @@ impl SignatureBatching { impl Drop for SignatureBatching { fn drop(&mut self) { + #[cfg(feature = "std")] + let panicking = std::thread::panicking(); + #[cfg(not(feature = "std"))] + let panicking = false; + // Sanity check. If user forgets to actually call `verify()`. - if !self.0 { + if !self.0 && !panicking { panic!("Signature verification has not been called before `SignatureBatching::drop`") } } @@ -885,4 +890,18 @@ mod tests { ); }); } + + #[test] + #[should_panic(expected = "Hey, I'm an error")] + fn batching_does_not_panics_while_thread_is_already_panicking() { + let mut ext = sp_state_machine::BasicExternalities::default(); + ext.register_extension( + sp_core::traits::TaskExecutorExt::new(sp_core::testing::TaskExecutor::new()), + ); + + ext.execute_with(|| { + let _batching = SignatureBatching::start(); + panic!("Hey, I'm an error"); + }); + } } From 56c589cfe7d5f7e831928664c777a0bb09876564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 22 Sep 2020 08:53:50 +0200 Subject: [PATCH 2/3] Update primitives/runtime/src/lib.rs Co-authored-by: David --- primitives/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index ece93568c900e..9b1c7e43a7601 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -893,7 +893,7 @@ mod tests { #[test] #[should_panic(expected = "Hey, I'm an error")] - fn batching_does_not_panics_while_thread_is_already_panicking() { + fn batching_does_not_panic_while_thread_is_already_panicking() { let mut ext = sp_state_machine::BasicExternalities::default(); ext.register_extension( sp_core::traits::TaskExecutorExt::new(sp_core::testing::TaskExecutor::new()), From f674cb9bfdc25f25313d4ecd9f8e340e41645199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 22 Sep 2020 09:04:03 +0200 Subject: [PATCH 3/3] Move function to `sp-std` --- primitives/runtime/src/lib.rs | 10 ++++------ primitives/std/with_std.rs | 4 ++++ primitives/std/without_std.rs | 9 +++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 9b1c7e43a7601..ee381d82bef81 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -794,13 +794,11 @@ impl SignatureBatching { impl Drop for SignatureBatching { fn drop(&mut self) { - #[cfg(feature = "std")] - let panicking = std::thread::panicking(); - #[cfg(not(feature = "std"))] - let panicking = false; - // Sanity check. If user forgets to actually call `verify()`. - if !self.0 && !panicking { + // + // We should not panic if the current thread is already panicking, + // because Rust otherwise aborts the process. + if !self.0 && !sp_std::thread::panicking() { panic!("Signature verification has not been called before `SignatureBatching::drop`") } } diff --git a/primitives/std/with_std.rs b/primitives/std/with_std.rs index e1994e764d23e..92e804b27e1d0 100644 --- a/primitives/std/with_std.rs +++ b/primitives/std/with_std.rs @@ -44,3 +44,7 @@ pub mod collections { pub use std::collections::btree_set; pub use std::collections::vec_deque; } + +pub mod thread { + pub use std::thread::panicking; +} diff --git a/primitives/std/without_std.rs b/primitives/std/without_std.rs index 09f7a1976cc02..3c130d547a1e4 100755 --- a/primitives/std/without_std.rs +++ b/primitives/std/without_std.rs @@ -53,3 +53,12 @@ pub mod borrow { pub use core::borrow::*; pub use alloc::borrow::*; } + +pub mod thread { + /// Returns if the current thread is panicking. + /// + /// In wasm this always returns `false`, as we abort on any panic. + pub fn panicking() -> bool { + false + } +}