Merge branch 'api-fix-pause'

3 files changed, 69 insertions, 13 deletions

diff --git a/mullvad-api/Cargo.toml b/mullvad-api/Cargo.toml
index c181694a4b..fe29981069 100644
--- a/mullvad-api/Cargo.toml
+++ b/mullvad-api/Cargo.toml
@@ -37,6 +37,10 @@ talpid-time = { path = "../talpid-time" }
 
 shadowsocks = { workspace = true,  features = [ "stream-cipher" ] }
 
+[dev-dependencies]
+talpid-time = { path = "../talpid-time", features = ["test"] }
+tokio = { workspace = true, features = ["test-util", "time"] }
+
 [build-dependencies]
 cbindgen = { version = "0.24.3", default-features = false }
 
diff --git a/mullvad-api/src/availability.rs b/mullvad-api/src/availability.rs
index 339aca8bca..0857a7812b 100644
--- a/mullvad-api/src/availability.rs
+++ b/mullvad-api/src/availability.rs
@@ -16,14 +16,6 @@ pub enum Error {
     Interrupted(#[from] broadcast::error::RecvError),
 }
 
-#[derive(PartialEq, Eq, Clone, Copy, Debug, Default)]
-pub struct State {
-    suspended: bool,
-    pause_background: bool,
-    offline: bool,
-    inactive: bool,
-}
-
 #[derive(Clone, Debug)]
 pub struct ApiAvailability(Arc<Mutex<ApiAvailabilityState>>);
 
@@ -34,6 +26,14 @@ struct ApiAvailabilityState {
     inactivity_timer: Option<tokio::task::JoinHandle<()>>,
 }
 
+#[derive(PartialEq, Eq, Clone, Copy, Debug, Default)]
+pub struct State {
+    suspended: bool,
+    pause_background: bool,
+    offline: bool,
+    inactive: bool,
+}
+
 impl State {
     pub const fn is_suspended(&self) -> bool {
         self.suspended
@@ -71,7 +71,7 @@ impl ApiAvailability {
     /// Reset task that automatically pauses API requests due inactivity,
     /// starting it if it's not currently running.
     pub fn reset_inactivity_timer(&self) {
-        let mut inner = self.0.lock().unwrap();
+        let mut inner = self.acquire();
         log::debug!("Restarting API inactivity check");
         inner.stop_inactivity_timer();
         let availability_handle = self.clone();
@@ -94,7 +94,7 @@ impl ApiAvailability {
     pub fn resume_background(&self) {
         let should_reset = {
             let mut inner = self.acquire();
-            inner.pause_background();
+            inner.resume_background();
             inner.inactivity_timer_running()
         };
         // Note: It is important that we do not hold on to the Mutex when calling `reset_inactivity_timer()`.
@@ -179,6 +179,12 @@ impl ApiAvailability {
     }
 }
 
+impl Default for ApiAvailability {
+    fn default() -> Self {
+        ApiAvailability::new(State::default())
+    }
+}
+
 impl ApiAvailabilityState {
     fn suspend(&mut self) {
         if !self.state.suspended {
@@ -237,6 +243,14 @@ impl ApiAvailabilityState {
         }
     }
 
+    fn resume_background(&mut self) {
+        if self.state.pause_background {
+            log::debug!("Resuming background API requests");
+            self.state.pause_background = false;
+            let _ = self.tx.send(self.state);
+        }
+    }
+
     fn stop_inactivity_timer(&mut self) {
         log::debug!("Stopping API inactivity check");
         if let Some(timer) = self.inactivity_timer.take() {
@@ -254,3 +268,39 @@ impl Drop for ApiAvailabilityState {
         self.stop_inactivity_timer();
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    /// Use mockable time for tests
+    pub use tokio::time::Duration;
+
+    // Note that all of these tests needs a tokio runtime. Creating an instance of [`ApiAvailability`] will implicitly
+    // spawn a tokio task.
+
+    /// Test that the inactivity timer starts in an expected state.
+    #[tokio::test(start_paused = true)]
+    async fn test_initially_active() {
+        // Start a new timer. It should *not* start as paused.
+        let timer = ApiAvailability::default();
+        assert!(
+            !timer.get_state().is_background_paused(),
+            "Inactivity timer should be active"
+        )
+    }
+
+    /// Test that the inactivity timer kicks in after [`INACTIVITY_TIME`] of inactivity.
+    #[tokio::test(start_paused = true)]
+    async fn test_inactivity() {
+        // Start a new timer. It should be marked as 'active'.
+        let timer = ApiAvailability::default();
+        // Elapse INACTIVITY_TIME (+ some slack because clocks)
+        const SLACK: Duration = Duration::from_secs(1);
+        talpid_time::sleep(INACTIVITY_TIME + SLACK).await;
+        // Check that the timer is now marked as 'inactive'
+        assert!(
+            timer.get_state().is_background_paused(),
+            "Inactivity timer should be inactive because 'INACTIVITY_TIME' has passed"
+        )
+    }
+}
diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index 8add11d30a..ebc1401a5f 100644
--- a/mullvad-api/src/lib.rs
+++ b/mullvad-api/src/lib.rs
@@ -340,7 +340,7 @@ impl Runtime {
         Runtime {
             handle,
             address_cache: AddressCache::with_static_addr(address),
-            api_availability: ApiAvailability::new(availability::State::default()),
+            api_availability: ApiAvailability::default(),
         }
     }
 
@@ -351,7 +351,7 @@ impl Runtime {
         Ok(Runtime {
             handle,
             address_cache: AddressCache::new(None)?,
-            api_availability: ApiAvailability::new(availability::State::default()),
+            api_availability: ApiAvailability::default(),
             #[cfg(target_os = "android")]
             socket_bypass_tx,
         })
@@ -396,10 +396,12 @@ impl Runtime {
             }
         };
 
+        let api_availability = ApiAvailability::default();
+
         Ok(Runtime {
             handle,
             address_cache,
-            api_availability: ApiAvailability::new(availability::State::default()),
+            api_availability,
             #[cfg(target_os = "android")]
             socket_bypass_tx,
         })