From e12a5159a817c2cc261482111e3867599e7680db Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 4 Apr 2018 11:50:28 +0200 Subject: [PATCH] Cleaner binary shutdown system (#8284) * Cleaner shutdown system when executing * Simplify set_exit_handler for Client * Minor change * Fix submodule --- ethcore/src/client/client.rs | 11 ++- parity/run.rs | 166 ++++++++++++++++++++--------------- 2 files changed, 100 insertions(+), 77 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index c7ad18dcb..31f8f2f25 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -220,7 +220,7 @@ pub struct Client { registrar_address: Option
, /// A closure to call when we want to restart the client - exit_handler: Mutex) + 'static + Send>>>, + exit_handler: Mutex>>, importer: Importer, } @@ -825,8 +825,11 @@ impl Client { self.notify.write().push(Arc::downgrade(&target)); } - /// Set a closure to call when we want to restart the client - pub fn set_exit_handler(&self, f: F) where F: Fn(bool, Option) + 'static + Send { + /// Set a closure to call when the client wants to be restarted. + /// + /// The parameter passed to the callback is the name of the new chain spec to use after + /// the restart. + pub fn set_exit_handler(&self, f: F) where F: Fn(String) + 'static + Send { *self.exit_handler.lock() = Some(Box::new(f)); } @@ -1625,7 +1628,7 @@ impl BlockChainClient for Client { return; } if let Some(ref h) = *self.exit_handler.lock() { - (*h)(true, Some(new_spec_name)); + (*h)(new_spec_name); } else { warn!("Not hypervised; cannot change chain."); } diff --git a/parity/run.rs b/parity/run.rs index 9b64a40d9..4c73fe63f 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::any::Any; use std::fmt; use std::sync::{Arc, Weak}; use std::time::{Duration, Instant}; @@ -182,7 +183,7 @@ impl ::local_store::NodeInfo for FullNodeInfo { type LightClient = ::light::client::Client<::light_helpers::EpochFetch>; // helper for light execution. -fn execute_light_impl(cmd: RunCmd, can_restart: bool, logger: Arc) -> Result<((bool, Option), Weak), String> { +fn execute_light_impl(cmd: RunCmd, logger: Arc) -> Result { use light::client as light_client; use ethsync::{LightSyncParams, LightSync, ManageNetwork}; use parking_lot::{Mutex, RwLock}; @@ -260,7 +261,7 @@ fn execute_light_impl(cmd: RunCmd, can_restart: bool, logger: Arc) -> Result<((bool, Option), Weak), String> { +fn execute_impl(cmd: RunCmd, logger: Arc, on_client_rq: Cr, + on_updater_rq: Rr) -> Result + where Cr: Fn(String) + 'static + Send, + Rr: Fn() + 'static + Send +{ // load spec let spec = cmd.spec.spec(&cmd.dirs.cache)?; @@ -854,7 +856,7 @@ pub fn execute_impl(cmd: RunCmd, can_restart: bool, logger: Arc) }); // the watcher must be kept alive. - let _watcher = match cmd.no_periodic_snapshot { + let watcher = match cmd.no_periodic_snapshot { true => None, false => { let sync = sync_provider.clone(); @@ -881,23 +883,58 @@ pub fn execute_impl(cmd: RunCmd, can_restart: bool, logger: Arc) open_dapp(&cmd.dapps_conf, &cmd.http_conf, &dapp)?; } - // Create a weak reference to the client so that we can wait on shutdown until it is dropped - let weak_client = Arc::downgrade(&client); + client.set_exit_handler(on_client_rq); + updater.set_exit_handler(on_updater_rq); - // Handle exit - let restart = wait_for_exit(Some(updater), Some(client), can_restart); + Ok(RunningClient::Full { + informant, + client, + keep_alive: Box::new((watcher, service, updater, ws_server, http_server, ipc_server, ui_server, secretstore_key_server, ipfs_server, event_loop)), + }) +} - info!("Finishing work, please wait..."); +enum RunningClient { + Light { + informant: Arc>, + client: Arc, + keep_alive: Box, + }, + Full { + informant: Arc>, + client: Arc, + keep_alive: Box, + }, +} - // drop this stuff as soon as exit detected. - drop((ws_server, http_server, ipc_server, ui_server, secretstore_key_server, ipfs_server, event_loop)); - - // to make sure timer does not spawn requests while shutdown is in progress - informant.shutdown(); - // just Arc is dropping here, to allow other reference release in its default time - drop(informant); - - Ok((restart, weak_client)) +impl RunningClient { + fn shutdown(self) { + match self { + RunningClient::Light { informant, client, keep_alive } => { + // Create a weak reference to the client so that we can wait on shutdown + // until it is dropped + let weak_client = Arc::downgrade(&client); + drop(keep_alive); + informant.shutdown(); + drop(informant); + drop(client); + wait_for_drop(weak_client); + }, + RunningClient::Full { informant, client, keep_alive } => { + info!("Finishing work, please wait..."); + // Create a weak reference to the client so that we can wait on shutdown + // until it is dropped + let weak_client = Arc::downgrade(&client); + // drop this stuff as soon as exit detected. + drop(keep_alive); + // to make sure timer does not spawn requests while shutdown is in progress + informant.shutdown(); + // just Arc is dropping here, to allow other reference release in its default time + drop(informant); + drop(client); + wait_for_drop(weak_client); + } + } + } } pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc) -> Result<(bool, Option), String> { @@ -917,18 +954,34 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc) -> R // increase max number of open files raise_fd_limit(); - fn wait(res: Result<((bool, Option), Weak), String>) -> Result<(bool, Option), String> { - res.map(|(restart, weak_client)| { - wait_for_drop(weak_client); - restart - }) - } + let exit = Arc::new((Mutex::new((false, None)), Condvar::new())); - if cmd.light { - wait(execute_light_impl(cmd, can_restart, logger)) + let running_client = if cmd.light { + execute_light_impl(cmd, logger)? + } else if can_restart { + let e1 = exit.clone(); + let e2 = exit.clone(); + execute_impl(cmd, logger, + move |new_chain: String| { *e1.0.lock() = (true, Some(new_chain)); e1.1.notify_all(); }, + move || { *e2.0.lock() = (true, None); e2.1.notify_all(); })? } else { - wait(execute_impl(cmd, can_restart, logger)) - } + trace!(target: "mode", "Not hypervised: not setting exit handlers."); + execute_impl(cmd, logger, move |_| {}, move || {})? + }; + + // Handle possible exits + CtrlC::set_handler({ + let e = exit.clone(); + move || { e.1.notify_all(); } + }); + + // Wait for signal + let mut l = exit.0.lock(); + let _ = exit.1.wait(&mut l); + + running_client.shutdown(); + + Ok(l.clone()) } #[cfg(not(windows))] @@ -1029,39 +1082,6 @@ fn build_create_account_hint(spec: &SpecType, keys: &str) -> String { format!("You can create an account via RPC, UI or `parity account new --chain {} --keys-path {}`.", spec, keys) } -fn wait_for_exit( - updater: Option>, - client: Option>, - can_restart: bool -) -> (bool, Option) { - let exit = Arc::new((Mutex::new((false, None)), Condvar::new())); - - // Handle possible exits - let e = exit.clone(); - CtrlC::set_handler(move || { e.1.notify_all(); }); - - if can_restart { - if let Some(updater) = updater { - // Handle updater wanting to restart us - let e = exit.clone(); - updater.set_exit_handler(move || { *e.0.lock() = (true, None); e.1.notify_all(); }); - } - - if let Some(client) = client { - // Handle updater wanting to restart us - let e = exit.clone(); - client.set_exit_handler(move |restart, new_chain: Option| { *e.0.lock() = (restart, new_chain); e.1.notify_all(); }); - } - } else { - trace!(target: "mode", "Not hypervised: not setting exit handlers."); - } - - // Wait for signal - let mut l = exit.0.lock(); - let _ = exit.1.wait(&mut l); - l.clone() -} - fn wait_for_drop(w: Weak) { let sleep_duration = Duration::from_secs(1); let warn_timeout = Duration::from_secs(60);