Merge branch 'move-api-address-updater' into main

6 files changed, 75 insertions, 74 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c713a7441..2707f71b2f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,8 @@ Line wrap the file at 100 chars.                                              Th
 - Fix connectivity issues that would occur when using quantum-resistant tunnels with an incorrectly
   configured MTU.
 - Fix custom list name validation by not allowing empty names.
+- Continual excessive attempts to update the API IP were made after testing access methods.
+- Fix pointless API access method rotations for concurrent requests.
 
 #### Linux
 - Fix Bash shell completions for subcommands in the CLI.
diff --git a/mullvad-api/src/address_cache.rs b/mullvad-api/src/address_cache.rs
index f164e15f09..aa9a155bec 100644
--- a/mullvad-api/src/address_cache.rs
+++ b/mullvad-api/src/address_cache.rs
@@ -1,3 +1,5 @@
+//! This module keeps track of the last known good API IP address and reads and stores it on disk.
+
 use super::API;
 use std::{io, net::SocketAddr, path::Path, sync::Arc};
 use tokio::{
diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index 6114bec90a..e78535dfc0 100644
--- a/mullvad-api/src/lib.rs
+++ b/mullvad-api/src/lib.rs
@@ -308,7 +308,7 @@ impl ApiEndpoint {
 /// A type that helps with the creation of API connections.
 pub struct Runtime {
     handle: tokio::runtime::Handle,
-    pub address_cache: AddressCache,
+    address_cache: AddressCache,
     api_availability: availability::ApiAvailability,
     #[cfg(target_os = "android")]
     socket_bypass_tx: Option<mpsc::Sender<SocketBypassRequest>>,
@@ -437,12 +437,7 @@ impl Runtime {
         let token_store = access::AccessTokenStore::new(service.clone());
         let factory = rest::RequestFactory::new(API.host(), Some(token_store));
 
-        rest::MullvadRestHandle::new(
-            service,
-            factory,
-            self.address_cache.clone(),
-            self.availability_handle(),
-        )
+        rest::MullvadRestHandle::new(service, factory, self.availability_handle())
     }
 
     /// This is only to be used in test code
@@ -456,12 +451,7 @@ impl Runtime {
         let token_store = access::AccessTokenStore::new(service.clone());
         let factory = rest::RequestFactory::new(hostname, Some(token_store));
 
-        rest::MullvadRestHandle::new(
-            service,
-            factory,
-            self.address_cache.clone(),
-            self.availability_handle(),
-        )
+        rest::MullvadRestHandle::new(service, factory, self.availability_handle())
     }
 
     /// Returns a new request service handle
@@ -481,6 +471,10 @@ impl Runtime {
     pub fn availability_handle(&self) -> ApiAvailabilityHandle {
         self.api_availability.handle()
     }
+
+    pub fn address_cache(&self) -> &AddressCache {
+        &self.address_cache
+    }
 }
 
 #[derive(Clone)]
diff --git a/mullvad-api/src/rest.rs b/mullvad-api/src/rest.rs
index e4a1235177..0fd826abc4 100644
--- a/mullvad-api/src/rest.rs
+++ b/mullvad-api/src/rest.rs
@@ -30,10 +30,6 @@ pub use hyper::StatusCode;
 
 const USER_AGENT: &str = "mullvad-app";
 
-const API_IP_CHECK_INITIAL: Duration = Duration::from_secs(15 * 60);
-const API_IP_CHECK_INTERVAL: Duration = Duration::from_secs(24 * 60 * 60);
-const API_IP_CHECK_ERROR_INTERVAL: Duration = Duration::from_secs(15 * 60);
-
 pub type Result<T> = std::result::Result<T, Error>;
 const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
 
@@ -616,68 +612,13 @@ impl MullvadRestHandle {
     pub(crate) fn new(
         service: RequestServiceHandle,
         factory: RequestFactory,
-        address_cache: AddressCache,
         availability: ApiAvailabilityHandle,
     ) -> Self {
-        let handle = Self {
+        Self {
             service,
             factory,
             availability,
-        };
-        #[cfg(feature = "api-override")]
-        {
-            if crate::API.disable_address_cache {
-                return handle;
-            }
         }
-        handle.spawn_api_address_fetcher(address_cache);
-        handle
-    }
-
-    fn spawn_api_address_fetcher(&self, address_cache: AddressCache) {
-        let handle = self.clone();
-        let availability = self.availability.clone();
-
-        tokio::spawn(async move {
-            let api_proxy = crate::ApiProxy::new(handle);
-            let mut next_delay = API_IP_CHECK_INITIAL;
-
-            loop {
-                talpid_time::sleep(next_delay).await;
-
-                if let Err(error) = availability.wait_background().await {
-                    log::error!("Failed while waiting for API: {}", error);
-                    continue;
-                }
-                match api_proxy.clone().get_api_addrs().await {
-                    Ok(new_addrs) => {
-                        if let Some(addr) = new_addrs.first() {
-                            log::debug!(
-                                "Fetched new API address {:?}. Fetching again in {} hours",
-                                addr,
-                                API_IP_CHECK_INTERVAL.as_secs() / (60 * 60)
-                            );
-                            if let Err(err) = address_cache.set_address(*addr).await {
-                                log::error!("Failed to save newly updated API address: {}", err);
-                            }
-                        } else {
-                            log::error!("API returned no API addresses");
-                        }
-
-                        next_delay = API_IP_CHECK_INTERVAL;
-                    }
-                    Err(err) => {
-                        log::error!(
-                            "Failed to fetch new API addresses: {}. Retrying in {} seconds",
-                            err,
-                            API_IP_CHECK_ERROR_INTERVAL.as_secs()
-                        );
-
-                        next_delay = API_IP_CHECK_ERROR_INTERVAL;
-                    }
-                }
-            }
-        });
     }
 
     pub fn service(&self) -> RequestServiceHandle {
diff --git a/mullvad-daemon/src/api_address_updater.rs b/mullvad-daemon/src/api_address_updater.rs
new file mode 100644
index 0000000000..533c039acf
--- /dev/null
+++ b/mullvad-daemon/src/api_address_updater.rs
@@ -0,0 +1,55 @@
+//! A small updater that keeps the API IP address cache up to date by fetching changes from the
+//! Mullvad API.
+use mullvad_api::{rest::MullvadRestHandle, AddressCache, ApiProxy};
+use std::time::Duration;
+
+const API_IP_CHECK_INITIAL: Duration = Duration::from_secs(15 * 60);
+const API_IP_CHECK_INTERVAL: Duration = Duration::from_secs(24 * 60 * 60);
+const API_IP_CHECK_ERROR_INTERVAL: Duration = Duration::from_secs(15 * 60);
+
+pub async fn run_api_address_fetcher(address_cache: AddressCache, handle: MullvadRestHandle) {
+    #[cfg(feature = "api-override")]
+    if API.disable_address_cache {
+        return;
+    }
+
+    let availability = handle.availability.clone();
+    let api_proxy = ApiProxy::new(handle);
+    let mut next_delay = API_IP_CHECK_INITIAL;
+
+    loop {
+        talpid_time::sleep(next_delay).await;
+
+        if let Err(error) = availability.wait_background().await {
+            log::error!("Failed while waiting for API: {}", error);
+            continue;
+        }
+        match api_proxy.clone().get_api_addrs().await {
+            Ok(new_addrs) => {
+                if let Some(addr) = new_addrs.first() {
+                    log::debug!(
+                        "Fetched new API address {:?}. Fetching again in {} hours",
+                        addr,
+                        API_IP_CHECK_INTERVAL.as_secs() / (60 * 60)
+                    );
+                    if let Err(err) = address_cache.set_address(*addr).await {
+                        log::error!("Failed to save newly updated API address: {}", err);
+                    }
+                } else {
+                    log::error!("API returned no API addresses");
+                }
+
+                next_delay = API_IP_CHECK_INTERVAL;
+            }
+            Err(err) => {
+                log::error!(
+                    "Failed to fetch new API addresses: {}. Retrying in {} seconds",
+                    err,
+                    API_IP_CHECK_ERROR_INTERVAL.as_secs()
+                );
+
+                next_delay = API_IP_CHECK_ERROR_INTERVAL;
+            }
+        }
+    }
+}
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index e446735df0..fde4fc534b 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -3,6 +3,7 @@
 mod access_method;
 pub mod account_history;
 mod api;
+mod api_address_updater;
 #[cfg(not(target_os = "android"))]
 mod cleanup;
 mod custom_list;
@@ -712,13 +713,19 @@ where
             relay_selector.clone(),
             settings.api_access_methods.clone(),
             internal_event_tx.to_specialized_sender(),
-            api_runtime.address_cache.clone().clone(),
+            api_runtime.address_cache().clone(),
         )
         .await
         .map_err(Error::ApiConnectionModeError)?;
 
         let api_handle = api_runtime.mullvad_rest_handle(access_mode_provider);
 
+        // Continually update the API IP
+        tokio::spawn(api_address_updater::run_api_address_fetcher(
+            api_runtime.address_cache().clone(),
+            api_handle.clone(),
+        ));
+
         let access_method_handle = access_mode_handler.clone();
         settings.register_change_listener(move |settings| {
             let handle = access_method_handle.clone();