fix gui: stop daemon gracefully

wizardsardine · Aug 22, 2023 · 63a221a · 63a221a
1 parent 8dd4710
commit 63a221a
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 127 deletions.
diff --git 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
@@ -58,9 +58,8 @@ impl<C: Client + Debug> Daemon for Lianad<C> {
  None
  }
 
- fn stop(&mut self) -> Result<(), DaemonError> {
- let _res: serde_json::value::Value = self.call("stop", Option::<Request>::None)?;
- Ok(())
+ fn stop(&self) {
+ unreachable!("GUI should not ask external client to stop")
  }
 
  fn get_info(&self) -> Result<GetInfoResult, DaemonError> {

diff --git 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<RwLock<DaemonHandle>>,
+ 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<EmbeddedDaemon, DaemonError> {
+ 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<GetInfoResult, DaemonError> {
- 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<GetAddressResult, DaemonError> {
- 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<ListCoinsResult, DaemonError> {
- 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<ListSpendResult, DaemonError> {
- 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<ListTransactionsResult, DaemonError> {
  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<ListTransactionsResult, DaemonError> {
- 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,54 +81,32 @@ impl Daemon for EmbeddedDaemon {
  feerate_vb: u64,
  ) -> Result<CreateSpendResult, DaemonError> {
  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()))
  }
 
  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()))
  }
 
  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<u16>,
  ) -> Result<Psbt, DaemonError> {
  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
@@ -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<model::GetInfoResult, DaemonError>;
  fn get_new_address(&self) -> Result<model::GetAddressResult, DaemonError>;
  fn list_coins(&self) -> Result<model::ListCoinsResult, DaemonError>;

diff --git 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<Arc<dyn Daemon + Sync
 
  let config = Config::from_file(Some(config_path)).map_err(Error::Config)?;
 
- let mut daemon = EmbeddedDaemon::new(config);
- daemon.start()?;
+ let daemon = EmbeddedDaemon::start(config)?;
 
  Ok(Arc::new(daemon))
 }