From 924df8e1d5440159306504c0ddbbee6f962cd7c5 Mon Sep 17 00:00:00 2001 From: edouardparis Date: Fri, 15 Mar 2024 15:24:29 +0100 Subject: [PATCH 1/2] bump liana:master --- gui/Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/Cargo.lock b/gui/Cargo.lock index 2f0d8bdd..6b2c88ba 100644 --- a/gui/Cargo.lock +++ b/gui/Cargo.lock @@ -2620,7 +2620,7 @@ dependencies = [ [[package]] name = "liana" version = "4.0.0" -source = "git+https://github.com/wizardsardine/liana?branch=master#dd29578c500ea7644154a5923bea9fa1db9b68e5" +source = "git+https://github.com/wizardsardine/liana?branch=master#2aa8874456bd8e43085c1496dedde68d3ce9d49a" dependencies = [ "backtrace", "bdk_coin_select", From a7f2bccb9a443c78a9788eedce06e2d753f7d11e Mon Sep 17 00:00:00 2001 From: edouardparis Date: Mon, 18 Mar 2024 18:21:44 +0100 Subject: [PATCH 2/2] gui: handle daemon stop error --- gui/src/app/mod.rs | 14 ++-- gui/src/daemon/client/mod.rs | 6 +- gui/src/daemon/embedded.rs | 136 +++++++++++++++++++++++------------ gui/src/daemon/mod.rs | 3 +- gui/src/installer/mod.rs | 7 +- gui/src/loader.rs | 7 +- 6 files changed, 115 insertions(+), 58 deletions(-) diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index c769f23a..097b077f 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -16,7 +16,7 @@ use std::sync::Arc; use std::time::Duration; use iced::{clipboard, time, Command, Subscription}; -use tracing::{info, warn}; +use tracing::{error, info, warn}; pub use liana::{config::Config as DaemonConfig, miniscript::bitcoin}; use liana_ui::widget::Element; @@ -217,8 +217,11 @@ impl App { pub fn stop(&mut self) { info!("Close requested"); if !self.daemon.is_external() { - self.daemon.stop(); - info!("Internal daemon stopped"); + if let Err(e) = self.daemon.stop() { + error!("{}", e); + } else { + info!("Internal daemon stopped"); + } if let Some(bitcoind) = &self.internal_bitcoind { bitcoind.stop(); } @@ -232,6 +235,9 @@ impl App { let datadir_path = self.cache.datadir_path.clone(); Command::perform( async move { + // we check every 10 second if the daemon poller is alive + daemon.is_alive()?; + let info = daemon.get_info()?; // todo: filter coins to only have current coins. let coins = daemon.list_coins()?; @@ -278,7 +284,7 @@ impl App { daemon_config_path: &PathBuf, cfg: DaemonConfig, ) -> Result<(), Error> { - self.daemon.stop(); + self.daemon.stop()?; let daemon = EmbeddedDaemon::start(cfg)?; self.daemon = Arc::new(daemon); diff --git a/gui/src/daemon/client/mod.rs b/gui/src/daemon/client/mod.rs index 7ec4c0c1..eed29071 100644 --- a/gui/src/daemon/client/mod.rs +++ b/gui/src/daemon/client/mod.rs @@ -61,7 +61,11 @@ impl Daemon for Lianad { None } - fn stop(&self) { + fn is_alive(&self) -> Result<(), DaemonError> { + Ok(()) + } + + fn stop(&self) -> Result<(), DaemonError> { unreachable!("GUI should not ask external client to stop") } diff --git a/gui/src/daemon/embedded.rs b/gui/src/daemon/embedded.rs index 5091bb28..0ccf16b1 100644 --- a/gui/src/daemon/embedded.rs +++ b/gui/src/daemon/embedded.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::sync::Mutex; use super::{model::*, Daemon, DaemonError}; use liana::{ @@ -10,24 +11,35 @@ use liana::{ pub struct EmbeddedDaemon { config: Config, - handle: DaemonHandle, + handle: Mutex>, } impl EmbeddedDaemon { pub fn start(config: Config) -> Result { let handle = DaemonHandle::start_default(config.clone()).map_err(DaemonError::Start)?; - Ok(Self { handle, config }) + Ok(Self { + handle: Mutex::new(Some(handle)), + config, + }) } - fn control(&self) -> Result<&DaemonControl, DaemonError> { - if self.handle.shutdown_complete() { - Err(DaemonError::DaemonStopped) - } else { - Ok(&self.handle.control) + pub fn command(&self, method: F) -> Result + where + F: FnOnce(&DaemonControl) -> Result, + { + match self.handle.lock()?.as_ref() { + Some(DaemonHandle::Controller { control, .. }) => method(control), + None => Err(DaemonError::DaemonStopped), } } } +impl From> for DaemonError { + fn from(value: std::sync::PoisonError) -> Self { + DaemonError::Unexpected(format!("Daemon panic: {}", value)) + } +} + impl std::fmt::Debug for EmbeddedDaemon { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("DaemonHandle").finish() @@ -43,30 +55,48 @@ impl Daemon for EmbeddedDaemon { Some(&self.config) } - 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)); + fn is_alive(&self) -> Result<(), DaemonError> { + let mut handle = self.handle.lock()?; + if let Some(h) = handle.as_ref() { + if h.is_alive() { + return Ok(()); + } } + // if the daemon poller is not alive, we try to terminate it to fetch the error. + if let Some(h) = handle.take() { + h.stop() + .map_err(|e| DaemonError::Unexpected(e.to_string()))?; + } + Ok(()) + } + + fn stop(&self) -> Result<(), DaemonError> { + let mut handle = self.handle.lock()?; + if let Some(h) = handle.take() { + h.stop() + .map_err(|e| DaemonError::Unexpected(e.to_string()))?; + } + Ok(()) } fn get_info(&self) -> Result { - Ok(self.control()?.get_info()) + self.command(|daemon| Ok(daemon.get_info())) } fn get_new_address(&self) -> Result { - Ok(self.control()?.get_new_address()) + self.command(|daemon| Ok(daemon.get_new_address())) } fn list_coins(&self) -> Result { - Ok(self.control()?.list_coins(&[], &[])) + self.command(|daemon| Ok(daemon.list_coins(&[], &[]))) } fn list_spend_txs(&self) -> Result { - self.control()? - .list_spend(None) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .list_spend(None) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn list_confirmed_txs( @@ -75,13 +105,11 @@ impl Daemon for EmbeddedDaemon { end: u32, limit: u64, ) -> Result { - Ok(self - .control()? - .list_confirmed_transactions(start, end, limit)) + self.command(|daemon| Ok(daemon.list_confirmed_transactions(start, end, limit))) } fn list_txs(&self, txids: &[Txid]) -> Result { - Ok(self.control()?.list_transactions(txids)) + self.command(|daemon| Ok(daemon.list_transactions(txids))) } fn create_spend_tx( @@ -91,9 +119,11 @@ impl Daemon for EmbeddedDaemon { feerate_vb: u64, change_address: Option>, ) -> Result { - self.control()? - .create_spend(destinations, coins_outpoints, feerate_vb, change_address) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .create_spend(destinations, coins_outpoints, feerate_vb, change_address) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn rbf_psbt( @@ -102,32 +132,42 @@ impl Daemon for EmbeddedDaemon { is_cancel: bool, feerate_vb: Option, ) -> Result { - self.control()? - .rbf_psbt(txid, is_cancel, feerate_vb) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .rbf_psbt(txid, is_cancel, feerate_vb) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn update_spend_tx(&self, psbt: &Psbt) -> Result<(), DaemonError> { - self.control()? - .update_spend(psbt.clone()) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .update_spend(psbt.clone()) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn delete_spend_tx(&self, txid: &Txid) -> Result<(), DaemonError> { - self.control()?.delete_spend(txid); - Ok(()) + self.command(|daemon| { + daemon.delete_spend(txid); + Ok(()) + }) } fn broadcast_spend_tx(&self, txid: &Txid) -> Result<(), DaemonError> { - self.control()? - .broadcast_spend(txid) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .broadcast_spend(txid) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn start_rescan(&self, t: u32) -> Result<(), DaemonError> { - self.control()? - .start_rescan(t) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + self.command(|daemon| { + daemon + .start_rescan(t) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn create_recovery( @@ -136,21 +176,25 @@ impl Daemon for EmbeddedDaemon { feerate_vb: u64, sequence: Option, ) -> Result { - self.control()? - .create_recovery(address, feerate_vb, sequence) - .map_err(|e| DaemonError::Unexpected(e.to_string())) - .map(|res| res.psbt) + self.command(|daemon| { + daemon + .create_recovery(address, feerate_vb, sequence) + .map(|res| res.psbt) + .map_err(|e| DaemonError::Unexpected(e.to_string())) + }) } fn get_labels( &self, items: &HashSet, ) -> Result, DaemonError> { - Ok(self.handle.control.get_labels(items).labels) + self.command(|daemon| Ok(daemon.get_labels(items).labels)) } fn update_labels(&self, items: &HashMap>) -> Result<(), DaemonError> { - self.handle.control.update_labels(items); - Ok(()) + self.command(|daemon| { + daemon.update_labels(items); + Ok(()) + }) } } diff --git a/gui/src/daemon/mod.rs b/gui/src/daemon/mod.rs index 0b7492a7..5edbc9b0 100644 --- a/gui/src/daemon/mod.rs +++ b/gui/src/daemon/mod.rs @@ -52,7 +52,8 @@ impl std::fmt::Display for DaemonError { pub trait Daemon: Debug { fn is_external(&self) -> bool; fn config(&self) -> Option<&Config>; - fn stop(&self); + fn is_alive(&self) -> Result<(), DaemonError>; + fn stop(&self) -> Result<(), DaemonError>; fn get_info(&self) -> Result; fn get_new_address(&self) -> Result; fn list_coins(&self) -> Result; diff --git a/gui/src/installer/mod.rs b/gui/src/installer/mod.rs index 99c66e77..77273709 100644 --- a/gui/src/installer/mod.rs +++ b/gui/src/installer/mod.rs @@ -260,10 +260,9 @@ impl Installer { pub fn daemon_check(cfg: liana::config::Config) -> Result<(), Error> { // Start Daemon to check correctness of installation match liana::DaemonHandle::start_default(cfg) { - Ok(daemon) => { - daemon.shutdown(); - Ok(()) - } + Ok(daemon) => daemon + .stop() + .map_err(|e| Error::Unexpected(format!("Failed to stop Liana daemon: {}", e))), Err(e) => Err(Error::Unexpected(format!( "Failed to start Liana daemon: {}", e diff --git a/gui/src/loader.rs b/gui/src/loader.rs index da9f8da8..0716485f 100644 --- a/gui/src/loader.rs +++ b/gui/src/loader.rs @@ -228,8 +228,11 @@ impl Loader { if let Step::Syncing { daemon, .. } = &mut self.step { if !daemon.is_external() { info!("Stopping internal daemon..."); - daemon.stop(); - info!("Internal daemon stopped"); + if let Err(e) = daemon.stop() { + warn!("Internal daemon failed to stop: {}", e); + } else { + info!("Internal daemon stopped"); + } } }