From 76bb74371f7d33603ac0864b9ecea014a3cc0cda Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Thu, 31 Aug 2023 12:50:50 +0200
Subject: [PATCH] gui: installer: more robust poll for cookie file at bitcoind
 startup

We would formerly wait a definite number of seconds for the cookie file
to appear. But this approach presents a fundamental issue: how long is
enough before we can be reasonably sure bitcoind will never start?

For instance on mainnet the checks performed at startup could take more
than the 3 seconds we would previously wait for. If it does we would
have incorrectly errored and let bitcoind run in the background.

Instead, assume bitcoind would exit if it errors. Until it does so,
continue polling for the cookie file. This approach also presents
drawbacks (for instance what if bitcoind is performing a very long
operation before creating the cookie file?), but the former approach
wouldn't be an acceptable solution in this case either. And the new one
is preferable as its failure scenario seems much less probable.
---
 gui/src/bitcoind.rs  | 41 +++++++++++++++++++++++++++++------------
 gui/src/utils/mod.rs | 16 ----------------
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index ef3acc5a..875aa039 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -4,6 +4,8 @@ use liana::{
 };
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
+use std::thread;
+use std::time;
 use tokio::sync::Mutex;
 
 use tracing::{info, warn};
@@ -187,24 +189,39 @@ impl Bitcoind {
             .spawn()
             .map_err(|e| StartInternalBitcoindError::CommandError(e.to_string()))?;
 
-        if !crate::utils::poll_for_file(&config.cookie_path, 200, 15) {
-            match process.wait_with_output() {
-                Err(e) => {
-                    tracing::error!("Error while waiting for bitcoind to finish: {}", e)
-                }
-                Ok(o) => {
-                    tracing::error!("Exit status: {}", o.status);
-                    tracing::error!("stderr: {}", String::from_utf8_lossy(&o.stderr));
+        // We've started bitcoind in the background, however it may fail to start for whatever
+        // reason. And we need its JSONRPC interface to be available to continue. Thus wait for it
+        // to have created the cookie file, regularly checking it did not fail to start.
+        loop {
+            match process.try_wait() {
+                Ok(None) => {}
+                Err(e) => log::error!("Error while trying to wait for bitcoind: {}", e),
+                Ok(Some(status)) => {
+                    log::error!("Bitcoind exited with status '{}'", status);
+                    match process.wait_with_output() {
+                        Err(e) => {
+                            tracing::error!("Error while waiting for bitcoind to finish: {}", e)
+                        }
+                        Ok(o) => {
+                            tracing::error!("stderr: {}", String::from_utf8_lossy(&o.stderr));
+                        }
+                    }
+                    return Err(StartInternalBitcoindError::CookieFileNotFound(
+                        config.cookie_path.to_string_lossy().into_owned(),
+                    ));
                 }
             }
-            return Err(StartInternalBitcoindError::CookieFileNotFound(
-                config.cookie_path.to_string_lossy().into_owned(),
-            ));
+            if config.cookie_path.exists() {
+                log::info!("Bitcoind seems to have successfully started.");
+                break;
+            }
+            log::info!("Waiting for bitcoind to start.");
+            thread::sleep(time::Duration::from_millis(500));
         }
+
         config.cookie_path = config.cookie_path.canonicalize().map_err(|e| {
             StartInternalBitcoindError::CouldNotCanonicalizeCookiePath(e.to_string())
         })?;
-
         liana::BitcoinD::new(&config, "internal_bitcoind_start".to_string())
             .map_err(|e| StartInternalBitcoindError::BitcoinDError(e.to_string()))?;
 
diff --git a/gui/src/utils/mod.rs b/gui/src/utils/mod.rs
index 55b5a4b2..084f16d9 100644
--- a/gui/src/utils/mod.rs
+++ b/gui/src/utils/mod.rs
@@ -1,21 +1,5 @@
-use std::path::Path;
-
 #[cfg(test)]
 pub mod sandbox;
 
 #[cfg(test)]
 pub mod mock;
-
-/// Polls for a file's existence at given interval up to a maximum number of polls.
-/// Returns `true` once file exists and otherwise `false`.
-pub fn poll_for_file(path: &Path, interval_millis: u64, max_polls: u16) -> bool {
-    for i in 0..max_polls {
-        if path.exists() {
-            return true;
-        }
-        if i < max_polls.saturating_sub(1) {
-            std::thread::sleep(std::time::Duration::from_millis(interval_millis));
-        }
-    }
-    false
-}