From f5fcdf51d048e6ccae2df9c850d3de6b2d02d21d Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 21 Aug 2018 09:39:16 +0200 Subject: [PATCH 1/2] Deadlock fix! --- ethcore/sync/src/light_sync/mod.rs | 23 +++++++++++++++++++---- parity/informant.rs | 20 ++++++++++---------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/ethcore/sync/src/light_sync/mod.rs b/ethcore/sync/src/light_sync/mod.rs index 32e3a0dbfde..9e140de34dd 100644 --- a/ethcore/sync/src/light_sync/mod.rs +++ b/ethcore/sync/src/light_sync/mod.rs @@ -644,7 +644,14 @@ pub trait SyncInfo { fn start_block(&self) -> u64; /// Whether major sync is underway. - fn is_major_importing(&self) -> bool; + #[inline] + fn is_major_importing(&self) -> bool { + self.is_major_importing_do_wait(true) + } + + /// Whether major sync is underway. + /// When not waiting on the state we return false by default + fn is_major_importing_do_wait(&self, wait: bool) -> bool; } impl SyncInfo for LightSync { @@ -656,14 +663,22 @@ impl SyncInfo for LightSync { self.start_block_number } - fn is_major_importing(&self) -> bool { + fn is_major_importing_do_wait(&self, wait: bool) -> bool { const EMPTY_QUEUE: usize = 3; if self.client.as_light_client().queue_info().unverified_queue_size > EMPTY_QUEUE { return true; } - - match *self.state.lock() { + let mg_state = if wait { + self.state.lock() + } else { + if let Some(mg_state) = self.state.try_lock() { + mg_state + } else { + return false; + } + }; + match *mg_state { SyncState::Idle => false, _ => true, } diff --git a/parity/informant.rs b/parity/informant.rs index d3489a52f3b..39f5f91e8c7 100644 --- a/parity/informant.rs +++ b/parity/informant.rs @@ -184,7 +184,7 @@ impl InformantData for LightNodeInformantData { fn executes_transactions(&self) -> bool { false } fn is_major_importing(&self) -> bool { - self.sync.is_major_importing() + self.sync.is_major_importing_do_wait(false) } fn report(&self) -> Report { @@ -422,15 +422,15 @@ impl LightChainNotify for Informant { if ripe { if let Some(header) = good.last().and_then(|h| client.block_header(BlockId::Hash(*h))) { info!(target: "import", "Imported {} {} ({} Mgas){}", - Colour::White.bold().paint(format!("#{}", header.number())), - Colour::White.bold().paint(format!("{}", header.hash())), - Colour::Yellow.bold().paint(format!("{:.2}", header.gas_used().low_u64() as f32 / 1000000f32)), - if good.len() > 1 { - format!(" + another {} header(s)", - Colour::Red.bold().paint(format!("{}", good.len() - 1))) - } else { - String::new() - } + Colour::White.bold().paint(format!("#{}", header.number())), + Colour::White.bold().paint(format!("{}", header.hash())), + Colour::Yellow.bold().paint(format!("{:.2}", header.gas_used().low_u64() as f32 / 1000000f32)), + if good.len() > 1 { + format!(" + another {} header(s)", + Colour::Red.bold().paint(format!("{}", good.len() - 1))) + } else { + String::new() + } ); *last_import = Instant::now(); } From 365e91a015829db90e0827b295224b2c0d133e40 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 23 Aug 2018 18:19:16 +0200 Subject: [PATCH 2/2] Rename trait function to `is_major_importing_no_sync` and keep technical on private. --- ethcore/sync/src/light_sync/mod.rs | 55 ++++++++++++++++-------------- parity/informant.rs | 2 +- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/ethcore/sync/src/light_sync/mod.rs b/ethcore/sync/src/light_sync/mod.rs index 9e140de34dd..4b67c3a476b 100644 --- a/ethcore/sync/src/light_sync/mod.rs +++ b/ethcore/sync/src/light_sync/mod.rs @@ -614,6 +614,27 @@ impl LightSync { }; } } + + fn is_major_importing_do_wait(&self, wait: bool) -> bool { + const EMPTY_QUEUE: usize = 3; + + if self.client.as_light_client().queue_info().unverified_queue_size > EMPTY_QUEUE { + return true; + } + let mg_state = if wait { + self.state.lock() + } else { + if let Some(mg_state) = self.state.try_lock() { + mg_state + } else { + return false; + } + }; + match *mg_state { + SyncState::Idle => false, + _ => true, + } + } } // public API @@ -644,14 +665,10 @@ pub trait SyncInfo { fn start_block(&self) -> u64; /// Whether major sync is underway. - #[inline] - fn is_major_importing(&self) -> bool { - self.is_major_importing_do_wait(true) - } + fn is_major_importing(&self) -> bool; - /// Whether major sync is underway. - /// When not waiting on the state we return false by default - fn is_major_importing_do_wait(&self, wait: bool) -> bool; + /// Whether major sync is underway, skipping some synchronization. + fn is_major_importing_no_sync(&self) -> bool; } impl SyncInfo for LightSync { @@ -663,24 +680,12 @@ impl SyncInfo for LightSync { self.start_block_number } - fn is_major_importing_do_wait(&self, wait: bool) -> bool { - const EMPTY_QUEUE: usize = 3; + fn is_major_importing(&self) -> bool { + self.is_major_importing_do_wait(true) + } - if self.client.as_light_client().queue_info().unverified_queue_size > EMPTY_QUEUE { - return true; - } - let mg_state = if wait { - self.state.lock() - } else { - if let Some(mg_state) = self.state.try_lock() { - mg_state - } else { - return false; - } - }; - match *mg_state { - SyncState::Idle => false, - _ => true, - } + fn is_major_importing_no_sync(&self) -> bool { + self.is_major_importing_do_wait(false) } + } diff --git a/parity/informant.rs b/parity/informant.rs index 39f5f91e8c7..ddc19146a8a 100644 --- a/parity/informant.rs +++ b/parity/informant.rs @@ -184,7 +184,7 @@ impl InformantData for LightNodeInformantData { fn executes_transactions(&self) -> bool { false } fn is_major_importing(&self) -> bool { - self.sync.is_major_importing_do_wait(false) + self.sync.is_major_importing_no_sync() } fn report(&self) -> Report {