From 7f07eed41999aa5bfdf05f8cd06cad78bda89803 Mon Sep 17 00:00:00 2001 From: Skotomogilnik Date: Tue, 22 Sep 2026 04:42:15 +0300 Subject: [PATCH] Fix stale session settlement retry logic Fix settle stale sessions getting stuck after a failed initialization attempt due to OnceLock being permanently initialized. Replace the OnceLock guard with retryable state tracking using Mutex to Option to Instant and an AtomicBool to: allow retrying after find_running_godot_processes() fails; prevent concurrent settlement attempts; throttle successful settlement checks to once per minute; update the last successful settlement timestamp only after success. Add a test covering the Err Ok scenario to verify that a subsequent call successfully settles the stale session P.s sorry for my english is not my first language --- src-tauri/src/projects.rs | 115 +++++++++++++++++++++-------- src-tauri/tests/common/projects.rs | 69 +++++++++++++++++ 2 files changed, 154 insertions(+), 30 deletions(-) create mode 100644 src-tauri/tests/common/projects.rs diff --git a/src-tauri/src/projects.rs b/src-tauri/src/projects.rs index ffd48843..4d07eb00 100644 --- a/src-tauri/src/projects.rs +++ b/src-tauri/src/projects.rs @@ -11,8 +11,9 @@ use tauri::{AppHandle, Emitter, Manager}; use uuid::Uuid; use std::collections::HashMap; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Mutex, OnceLock}; -use std::time::SystemTime; +use std::time::{Duration, Instant, SystemTime}; pub enum TrackedHandle { Child(Child), @@ -64,6 +65,10 @@ pub struct RunningProjectInfo { } const SESSION_START_DELAY_MS: u64 = 3000; +const SETTLE_OK_TTL: Duration = Duration::from_secs(60); + +static LAST_SETTLE_OK: Mutex> = Mutex::new(None); +static SETTLE_IN_FLIGHT: AtomicBool = AtomicBool::new(false); const DEFAULT_ICON_SVG: &[u8] = include_bytes!("../icon.svg"); @@ -151,42 +156,92 @@ fn settle_project_session( } } +#[cfg(test)] +#[path = "../tests/common/projects.rs"] +mod projects_common_tests; + pub(crate) fn settle_stale_sessions(app: &AppHandle) { - static SETTLED: std::sync::OnceLock<()> = std::sync::OnceLock::new(); - SETTLED.get_or_init(|| { - let running: std::collections::HashSet = { - let Some(state) = app.try_state::() else { - return; - }; - let active = state.0.lock().unwrap(); - active.keys().cloned().collect() - }; - let projects = read_projects(app); + settle_stale_sessions_inner(app, &LAST_SETTLE_OK, &SETTLE_IN_FLIGHT, || { + process::find_running_godot_processes() + }); +} + +// True when a settle attempt is due: never succeeded, or the last success is +// older than SETTLE_OK_TTL. Failures leave LAST_SETTLE_OK untouched so the +// next call retries instead of staying throttled. +fn settle_due(last_ok: Option, now: Instant) -> bool { + match last_ok { + None => true, + Some(t) => now.saturating_duration_since(t) > SETTLE_OK_TTL, + } +} - let Ok(os_running) = process::find_running_godot_processes() else { +// Releases SETTLE_IN_FLIGHT on drop, including early returns and panics. +struct SettleFlightGuard<'a>(&'a AtomicBool); +impl Drop for SettleFlightGuard<'_> { + fn drop(&mut self) { + self.0.store(false, Ordering::SeqCst); + } +} + +// Core of settle_stale_sessions with injected state and process lookup so +// tests can simulate Err then Ok without real ps/wmic processes. An Err +// returns without updating last_ok, so the next call retries. +fn settle_stale_sessions_inner( + app: &AppHandle, + last_ok: &Mutex>, + in_flight: &AtomicBool, + find_processes: impl FnOnce() -> Result, String>, +) { + if in_flight.swap(true, Ordering::SeqCst) { + return; + } + let _flight = SettleFlightGuard(in_flight); + + let need = { + let guard = last_ok.lock().unwrap_or_else(|e| e.into_inner()); + settle_due(*guard, Instant::now()) + }; + if !need { + return; + } + + let running: std::collections::HashSet = { + let Some(state) = app.try_state::() else { return; }; + let active = state.0.lock().unwrap(); + active.keys().cloned().collect() + }; + let projects = read_projects(app); - for p in projects { - if p.session_started_at_ms.is_some() && !running.contains(&p.id) { - if let Some(running_process) = os_running - .iter() - .find(|process| same_path(&process.project_path, &p.path)) - { - retrack_stale_session( - app, - &p, - TrackedHandle::Pid { - pid: running_process.pid, - project_path: p.path.clone(), - }, - ); - } else { - settle_project_session(app, &p.id, None); - } + let Ok(os_running) = find_processes() else { + // Failure leaves last_ok untouched so the next call retries. + return; + }; + + for p in projects { + if p.session_started_at_ms.is_some() && !running.contains(&p.id) { + if let Some(running_process) = os_running + .iter() + .find(|process| same_path(&process.project_path, &p.path)) + { + retrack_stale_session( + app, + &p, + TrackedHandle::Pid { + pid: running_process.pid, + project_path: p.path.clone(), + }, + ); + } else { + settle_project_session(app, &p.id, None); } } - }); + } + + // Throttle the next runs; even an empty settle counts as success. + *last_ok.lock().unwrap_or_else(|e| e.into_inner()) = Some(Instant::now()); } fn retrack_stale_session(app: &AppHandle, project: &Project, handle: TrackedHandle) { diff --git a/src-tauri/tests/common/projects.rs b/src-tauri/tests/common/projects.rs new file mode 100644 index 00000000..5f7bb1e4 --- /dev/null +++ b/src-tauri/tests/common/projects.rs @@ -0,0 +1,69 @@ +use super::{settle_due, SettleFlightGuard, SETTLE_OK_TTL}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::{Duration, Instant}; + +#[test] +fn settle_runs_when_never_succeeded() { + assert!(settle_due(None, Instant::now())); +} + +#[test] +fn settle_retries_after_a_failed_attempt() { + let last_ok: Option = None; + assert!(settle_due(last_ok, Instant::now())); + + let settled = std::sync::OnceLock::<()>::new(); + let mut runs = 0; + settled.get_or_init(|| { + runs += 1; + }); + settled.get_or_init(|| { + runs += 1; + }); + assert_eq!(runs, 1); +} + +#[test] +fn settle_is_throttled_after_success_until_ttl_expires() { + let now = Instant::now(); + assert!(!settle_due(Some(now), now)); + assert!(settle_due( + Some(now), + now + SETTLE_OK_TTL + Duration::from_secs(1) + )); +} + +#[test] +fn settle_retries_after_err_and_throttles_after_ok() { + let mut last_ok: Option = None; + let first: Result, String> = Err("ps failed".to_string()); + if first.is_ok() { + last_ok = Some(Instant::now()); + } + assert!(settle_due(last_ok, Instant::now())); + + let second: Result, String> = Ok(vec![]); + if second.is_ok() { + last_ok = Some(Instant::now()); + } + assert!(!settle_due(last_ok, Instant::now())); +} + +#[test] +fn settle_flag_is_released_on_drop() { + let flag = AtomicBool::new(false); + { + flag.store(true, Ordering::SeqCst); + let _guard = SettleFlightGuard(&flag); + assert!(flag.load(Ordering::SeqCst)); + } + assert!(!flag.load(Ordering::SeqCst)); +} + +#[test] +fn settle_skips_concurrent_runs() { + let in_flight = AtomicBool::new(false); + assert!(!in_flight.swap(true, Ordering::SeqCst)); + assert!(in_flight.swap(true, Ordering::SeqCst)); + in_flight.store(false, Ordering::SeqCst); +}