From a1c716611769e1cdaa08b2b6a458052de2d9a003 Mon Sep 17 00:00:00 2001 From: edouard Date: Tue, 22 Aug 2023 15:00:11 +0200 Subject: [PATCH] fix gui: stop daemon gracefully --- gui/Cargo.lock | 2 +- gui/Cargo.toml | 2 +- gui/src/app/mod.rs | 18 ++---- gui/src/daemon/client/mod.rs | 5 +- gui/src/daemon/embedded.rs | 116 +++++------------------------------ gui/src/daemon/mod.rs | 5 +- gui/src/loader.rs | 10 +-- 7 files changed, 29 insertions(+), 129 deletions(-) diff --git a/gui/Cargo.lock b/gui/Cargo.lock index 5d6e207ab..94090710c 100644 --- a/gui/Cargo.lock +++ b/gui/Cargo.lock @@ -1919,7 +1919,7 @@ dependencies = [ [[package]] name = "liana" version = "1.0.0" -source = "git+https://github.com/wizardsardine/liana?branch=master#6c969aeacdde28a84f0a04060b280343a1e22afe" +source = "git+https://github.com/wizardsardine/liana?branch=master#05c9b580dba1933ffcc7efcc188dc75fdff211ba" dependencies = [ "backtrace", "bip39", diff --git a/gui/Cargo.toml b/gui/Cargo.toml index b67e180c3..250ac2615 100644 --- a/gui/Cargo.toml +++ b/gui/Cargo.toml @@ -15,7 +15,7 @@ path = "src/main.rs" [dependencies] async-hwi = "0.0.10" -liana = { git = "https://github.com/wizardsardine/liana", branch = "master", default-features = false } +liana = { git = "https://github.com/wizardsardine/liana", branch = "master", default-features = false, features = ["nonblocking_shutdown"] } liana_ui = { path = "ui" } backtrace = "0.3" base64 = "0.13" diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index 164f06fa1..d5ce721a4 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -31,7 +31,7 @@ use state::{ use crate::{ app::{cache::Cache, error::Error, menu::Menu, wallet::Wallet}, - daemon::Daemon, + daemon::{embedded::EmbeddedDaemon, Daemon}, }; pub struct App { @@ -113,11 +113,8 @@ impl App { pub fn stop(&mut self) { info!("Close requested"); if !self.daemon.is_external() { - info!("Stopping internal daemon..."); - if let Some(d) = Arc::get_mut(&mut self.daemon) { - d.stop().expect("Daemon is internal"); - info!("Internal daemon stopped"); - } + self.daemon.stop(); + info!("Internal daemon stopped"); } } @@ -171,12 +168,9 @@ impl App { daemon_config_path: &PathBuf, cfg: DaemonConfig, ) -> Result<(), Error> { - loop { - if let Some(daemon) = Arc::get_mut(&mut self.daemon) { - daemon.load_config(cfg)?; - break; - } - } + self.daemon.stop(); + let daemon = EmbeddedDaemon::start(cfg)?; + self.daemon = Arc::new(daemon); let mut daemon_config_file = OpenOptions::new() .write(true) diff --git a/gui/src/daemon/client/mod.rs b/gui/src/daemon/client/mod.rs index 956aa941d..6f4e11963 100644 --- a/gui/src/daemon/client/mod.rs +++ b/gui/src/daemon/client/mod.rs @@ -58,9 +58,8 @@ impl Daemon for Lianad { None } - fn stop(&mut self) -> Result<(), DaemonError> { - let _res: serde_json::value::Value = self.call("stop", Option::::None)?; - Ok(()) + fn stop(&self) { + unreachable!("GUI should not ask external client to stop") } fn get_info(&self) -> Result { diff --git a/gui/src/daemon/embedded.rs b/gui/src/daemon/embedded.rs index abbabe02d..b5d39989d 100644 --- a/gui/src/daemon/embedded.rs +++ b/gui/src/daemon/embedded.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::sync::RwLock; use super::{model::*, Daemon, DaemonError}; use liana::{ @@ -10,22 +9,13 @@ use liana::{ pub struct EmbeddedDaemon { config: Config, - handle: Option>, + handle: DaemonHandle, } impl EmbeddedDaemon { - pub fn new(config: Config) -> Self { - Self { - config, - handle: None, - } - } - - pub fn start(&mut self) -> Result<(), DaemonError> { - let handle = - DaemonHandle::start_default(self.config.clone()).map_err(DaemonError::Start)?; - self.handle = Some(RwLock::new(handle)); - Ok(()) + pub fn start(config: Config) -> Result { + let handle = DaemonHandle::start_default(config.clone()).map_err(DaemonError::Start)?; + Ok(Self { handle, config }) } } @@ -40,71 +30,32 @@ impl Daemon for EmbeddedDaemon { false } - fn load_config(&mut self, cfg: Config) -> Result<(), DaemonError> { - if self.handle.is_none() { - return Ok(()); - } - - let next = DaemonHandle::start_default(cfg).map_err(DaemonError::Start)?; - self.handle.take().unwrap().into_inner().unwrap().shutdown(); - self.handle = Some(RwLock::new(next)); - Ok(()) - } - fn config(&self) -> Option<&Config> { Some(&self.config) } - fn stop(&mut self) -> Result<(), DaemonError> { - if let Some(h) = self.handle.take() { - let handle = h.into_inner().unwrap(); - handle.shutdown(); + fn stop(&self) { + self.handle.trigger_shutdown(); + while !self.handle.shutdown_complete() { + tracing::debug!("Waiting daemon to shutdown"); + std::thread::sleep(std::time::Duration::from_millis(500)); } - Ok(()) } fn get_info(&self) -> Result { - Ok(self - .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .get_info()) + Ok(self.handle.control.get_info()) } fn get_new_address(&self) -> Result { - Ok(self - .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .get_new_address()) + Ok(self.handle.control.get_new_address()) } fn list_coins(&self) -> Result { - Ok(self - .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .list_coins()) + Ok(self.handle.control.list_coins()) } fn list_spend_txs(&self) -> Result { - Ok(self - .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .list_spend()) + Ok(self.handle.control.list_spend()) } fn list_confirmed_txs( @@ -115,23 +66,12 @@ impl Daemon for EmbeddedDaemon { ) -> Result { Ok(self .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .list_confirmed_transactions(start, end, limit)) } fn list_txs(&self, txids: &[Txid]) -> Result { - Ok(self - .handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .list_transactions(txids)) + Ok(self.handle.control.list_transactions(txids)) } fn create_spend_tx( @@ -141,10 +81,6 @@ impl Daemon for EmbeddedDaemon { feerate_vb: u64, ) -> Result { self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .create_spend(destinations, coins_outpoints, feerate_vb) .map_err(|e| DaemonError::Unexpected(e.to_string())) @@ -152,32 +88,18 @@ impl Daemon for EmbeddedDaemon { fn update_spend_tx(&self, psbt: &Psbt) -> Result<(), DaemonError> { self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .update_spend(psbt.clone()) .map_err(|e| DaemonError::Unexpected(e.to_string())) } fn delete_spend_tx(&self, txid: &Txid) -> Result<(), DaemonError> { - self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() - .control - .delete_spend(txid); + self.handle.control.delete_spend(txid); Ok(()) } fn broadcast_spend_tx(&self, txid: &Txid) -> Result<(), DaemonError> { self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .broadcast_spend(txid) .map_err(|e| DaemonError::Unexpected(e.to_string())) @@ -185,10 +107,6 @@ impl Daemon for EmbeddedDaemon { fn start_rescan(&self, t: u32) -> Result<(), DaemonError> { self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .start_rescan(t) .map_err(|e| DaemonError::Unexpected(e.to_string())) @@ -201,10 +119,6 @@ impl Daemon for EmbeddedDaemon { sequence: Option, ) -> Result { self.handle - .as_ref() - .ok_or(DaemonError::NoAnswer)? - .read() - .unwrap() .control .create_recovery(address, feerate_vb, sequence) .map_err(|e| DaemonError::Unexpected(e.to_string())) diff --git a/gui/src/daemon/mod.rs b/gui/src/daemon/mod.rs index 9f1ee813f..f768ecaf3 100644 --- a/gui/src/daemon/mod.rs +++ b/gui/src/daemon/mod.rs @@ -43,11 +43,8 @@ impl std::fmt::Display for DaemonError { pub trait Daemon: Debug { fn is_external(&self) -> bool; - fn load_config(&mut self, _cfg: Config) -> Result<(), DaemonError> { - Ok(()) - } fn config(&self) -> Option<&Config>; - fn stop(&mut self) -> Result<(), DaemonError>; + fn stop(&self); fn get_info(&self) -> Result; fn get_new_address(&self) -> Result; fn list_coins(&self) -> Result; diff --git a/gui/src/loader.rs b/gui/src/loader.rs index b35a3fc46..fb20aebbe 100644 --- a/gui/src/loader.rs +++ b/gui/src/loader.rs @@ -171,11 +171,8 @@ impl Loader { if let Step::Syncing { daemon, .. } = &mut self.step { if !daemon.is_external() { info!("Stopping internal daemon..."); - if let Some(d) = Arc::get_mut(daemon) { - d.stop().expect("Daemon is internal"); - info!("Internal daemon stopped"); - } else { - } + daemon.stop(); + info!("Internal daemon stopped"); } } } @@ -335,8 +332,7 @@ pub async fn start_daemon(config_path: PathBuf) -> Result