From 0d121dd51a84d854d65056e7441a9fb15d607dd5 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Wed, 10 Feb 2016 14:49:31 +0100 Subject: [PATCH] Removing unecessary locks causing dead-locks --- ethcore/src/block_queue.rs | 7 +++---- ethcore/src/client.rs | 6 +++--- util/src/io/service.rs | 5 ++--- util/src/io/worker.rs | 7 +++---- util/src/panics.rs | 34 +++++++++++++--------------------- 5 files changed, 24 insertions(+), 35 deletions(-) diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index 59de4403b..389435a61 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -60,7 +60,7 @@ impl BlockQueueInfo { /// A queue of blocks. Sits between network or other I/O and the BlockChain. /// Sorts them ready for blockchain insertion. pub struct BlockQueue { - panic_handler: SafeStringPanicHandler, + panic_handler: Arc, engine: Arc>, more_to_verify: Arc, verification: Arc>, @@ -115,7 +115,7 @@ impl BlockQueue { let ready_signal = Arc::new(QueueSignal { signalled: AtomicBool::new(false), message_channel: message_channel }); let deleting = Arc::new(AtomicBool::new(false)); let empty = Arc::new(Condvar::new()); - let panic_handler = StringPanicHandler::new_thread_safe(); + let panic_handler = StringPanicHandler::new_arc(); let mut verifiers: Vec> = Vec::new(); let thread_count = max(::num_cpus::get(), 3) - 2; @@ -131,8 +131,7 @@ impl BlockQueue { thread::Builder::new() .name(format!("Verifier #{}", i)) .spawn(move || { - let mut panic = panic_handler.lock().unwrap(); - panic.catch_panic(move || { + panic_handler.catch_panic(move || { BlockQueue::verify(verification, engine, more_to_verify, ready_signal, deleting, empty) }).unwrap() }) diff --git a/ethcore/src/client.rs b/ethcore/src/client.rs index 3596d56f9..8a3b18d5c 100644 --- a/ethcore/src/client.rs +++ b/ethcore/src/client.rs @@ -162,7 +162,7 @@ pub struct Client { block_queue: RwLock, report: RwLock, import_lock: Mutex<()>, - panic_handler: SafeStringPanicHandler, + panic_handler: Arc, } const HISTORY: u64 = 1000; @@ -211,9 +211,9 @@ impl Client { } let block_queue = BlockQueue::new(engine.clone(), message_channel); - let panic_handler = StringPanicHandler::new_thread_safe(); + let panic_handler = StringPanicHandler::new_arc(); let panic = panic_handler.clone(); - block_queue.on_panic(move |t: &String| panic.lock().unwrap().notify_all(t)); + block_queue.on_panic(move |t: &String| panic.notify_all(t)); Ok(Arc::new(Client { chain: chain, diff --git a/util/src/io/service.rs b/util/src/io/service.rs index 1f4eeea09..f65619a66 100644 --- a/util/src/io/service.rs +++ b/util/src/io/service.rs @@ -307,7 +307,7 @@ impl IoChannel where Message: Send + Clone { /// General IO Service. Starts an event loop and dispatches IO requests. /// 'Message' is a notification message type pub struct IoService where Message: Send + Sync + Clone + 'static { - panic_handler: SafeStringPanicHandler, + panic_handler: Arc, thread: Option>, host_channel: Sender>, } @@ -321,12 +321,11 @@ impl MayPanic for IoService where Message: Send + Sync impl IoService where Message: Send + Sync + Clone + 'static { /// Starts IO event loop pub fn start() -> Result, UtilError> { - let panic_handler = StringPanicHandler::new_thread_safe(); + let panic_handler = StringPanicHandler::new_arc(); let mut event_loop = EventLoop::new().unwrap(); let channel = event_loop.channel(); let panic = panic_handler.clone(); let thread = thread::spawn(move || { - let mut panic = panic.lock().unwrap(); panic.catch_panic(move || { IoManager::::start(&mut event_loop).unwrap(); }).unwrap() diff --git a/util/src/io/worker.rs b/util/src/io/worker.rs index 84979140b..33bb76bd7 100644 --- a/util/src/io/worker.rs +++ b/util/src/io/worker.rs @@ -44,7 +44,7 @@ pub struct Worker { thread: Option>, wait: Arc, deleting: Arc, - panic_handler: SafeStringPanicHandler, + panic_handler: Arc, } impl Worker { @@ -55,7 +55,7 @@ impl Worker { wait: Arc, wait_mutex: Arc>) -> Worker where Message: Send + Sync + Clone + 'static { - let panic_handler = StringPanicHandler::new_thread_safe(); + let panic_handler = StringPanicHandler::new_arc(); let deleting = Arc::new(AtomicBool::new(false)); let mut worker = Worker { panic_handler: panic_handler.clone(), @@ -66,8 +66,7 @@ impl Worker { let panic_handler = panic_handler.clone(); worker.thread = Some(thread::Builder::new().name(format!("IO Worker #{}", index)).spawn( move || { - let mut panic = panic_handler.lock().unwrap(); - panic.catch_panic(move || { + panic_handler.catch_panic(move || { Worker::work_loop(stealer, channel.clone(), wait, wait_mutex.clone(), deleting) }).unwrap() }) diff --git a/util/src/panics.rs b/util/src/panics.rs index dee5f3076..b618903b2 100644 --- a/util/src/panics.rs +++ b/util/src/panics.rs @@ -43,17 +43,9 @@ pub trait MayPanic { pub trait PanicHandler> : MayPanic{ fn with_converter(converter: C) -> Self; - fn catch_panic(&mut self, g: G) -> thread::Result + fn catch_panic(&self, g: G) -> thread::Result where G: FnOnce() -> R + Send + 'static; - fn notify_all(&mut self, &T); -} - -pub type SafeStringPanicHandler = Arc>; - -impl MayPanic for SafeStringPanicHandler { - fn on_panic(&self, closure: F) where F: OnPanicListener { - self.lock().unwrap().on_panic(closure); - } + fn notify_all(&self, &T); } pub struct StringConverter; @@ -84,7 +76,7 @@ impl PanicHandler for BasePanicHandler #[allow(deprecated)] // TODO [todr] catch_panic is deprecated but panic::recover has different bounds (not allowing mutex) - fn catch_panic(&mut self, g: G) -> thread::Result where G: FnOnce() -> R + Send + 'static { + fn catch_panic(&self, g: G) -> thread::Result where G: FnOnce() -> R + Send + 'static { let result = thread::catch_panic(g); if let Err(ref e) = result { @@ -97,7 +89,7 @@ impl PanicHandler for BasePanicHandler result } - fn notify_all(&mut self, r: &T) { + fn notify_all(&self, r: &T) { let mut listeners = self.listeners.lock().unwrap(); for listener in listeners.deref_mut() { listener.call(r); @@ -118,8 +110,8 @@ pub struct StringPanicHandler { } impl StringPanicHandler { - pub fn new_thread_safe() -> SafeStringPanicHandler { - Arc::new(Mutex::new(Self::new())) + pub fn new_arc() -> Arc { + Arc::new(Self::new()) } pub fn new () -> Self { @@ -135,11 +127,11 @@ impl PanicHandler for StringPanicHandler { } } - fn catch_panic(&mut self, g: G) -> thread::Result where G: FnOnce() -> R + Send + 'static { + fn catch_panic(&self, g: G) -> thread::Result where G: FnOnce() -> R + Send + 'static { self.handler.catch_panic(g) } - fn notify_all(&mut self, r: &String) { + fn notify_all(&self, r: &String) { self.handler.notify_all(r); } } @@ -157,7 +149,7 @@ fn should_notify_listeners_about_panic () { // given let invocations = Arc::new(RwLock::new(vec![])); let i = invocations.clone(); - let mut p = StringPanicHandler::new(); + let p = StringPanicHandler::new(); p.on_panic(move |t: &String| i.write().unwrap().push(t.clone())); // when @@ -173,7 +165,7 @@ fn should_notify_listeners_about_panic_when_string_is_dynamic () { // given let invocations = Arc::new(RwLock::new(vec![])); let i = invocations.clone(); - let mut p = StringPanicHandler::new(); + let p = StringPanicHandler::new(); p.on_panic(move |t: &String| i.write().unwrap().push(t.clone())); // when @@ -191,7 +183,7 @@ fn should_notify_listeners_about_panic_in_other_thread () { // given let invocations = Arc::new(RwLock::new(vec![])); let i = invocations.clone(); - let mut p = StringPanicHandler::new(); + let p = StringPanicHandler::new(); p.on_panic(move |t: &String| i.write().unwrap().push(t.clone())); // when @@ -210,10 +202,10 @@ use std::sync::RwLock; // given let invocations = Arc::new(RwLock::new(vec![])); let i = invocations.clone(); - let mut p = StringPanicHandler::new(); + let p = StringPanicHandler::new(); p.on_panic(move |t: &String| i.write().unwrap().push(t.clone())); - let mut p2 = StringPanicHandler::new(); + let p2 = StringPanicHandler::new(); p2.on_panic(move |t: &String| p.notify_all(t)); // when