From 6a444ebec7562e177aa748b83d6e6ad7000f70f3 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 17:48:09 -0500 Subject: [PATCH] fix(installer): release the update marker on success, not just on drop The updater keeps a Tauri/Cocoa event loop alive while it relaunches the desktop, and that loop can outlive app.exit(0). Relying on Drop alone left a *successful* update looking active -- a live pid holding a fresh marker -- which blocked desktop startup and, now that the marker is also the cross-process update lock, every subsequent updater until the age ceiling expired. Release explicitly once all install-tree mutations are done, before the relaunch. complete() is idempotent so Drop still covers the failure and panic paths. Arms a process-exit fallback so a wedged event loop cannot leave a finished updater lingering as a live pid. Co-authored-by: nateEc --- .../src-tauri/src/update.rs | 62 +++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/apps/bootstrap-installer/src-tauri/src/update.rs b/apps/bootstrap-installer/src-tauri/src/update.rs index f689bb6f0c..63a5bfe8d7 100644 --- a/apps/bootstrap-installer/src-tauri/src/update.rs +++ b/apps/bootstrap-installer/src-tauri/src/update.rs @@ -211,21 +211,33 @@ impl UpdateMarkerGuard { } Ok(Self { path, owned: true }) } -} -impl Drop for UpdateMarkerGuard { - fn drop(&mut self) { + /// Release the marker as soon as every mutating stage has completed. + /// + /// The updater still owns a Tauri/Cocoa event loop while it relaunches the + /// desktop, and that loop can outlive `app.exit(0)`. Relying on `Drop` + /// alone therefore leaves a *successful* update looking active — a live + /// pid holding a fresh marker — which blocks desktop startup and every + /// other updater for the full age ceiling. Idempotent: `Drop` still runs + /// and tolerates an already-removed marker. + fn complete(&self) { if !self.owned { return; } if let Err(err) = std::fs::remove_file(&self.path) { if err.kind() != std::io::ErrorKind::NotFound { - tracing::warn!(path = ?self.path, %err, "could not remove update-in-progress marker"); + tracing::warn!(path = ?self.path, %err, "could not remove completed update marker"); } } } } +impl Drop for UpdateMarkerGuard { + fn drop(&mut self) { + self.complete(); + } +} + async fn run_update(app: AppHandle) -> Result<()> { let hermes_home = crate::paths::hermes_home(); let install_root = hermes_home.join("hermes-agent"); @@ -569,6 +581,12 @@ async fn run_update(app: AppHandle) -> Result<()> { marker: None, }, ); + // Every install-tree mutation is finished. Release the lock BEFORE the + // relaunch: this process can stay wedged in its native event loop even + // after a successful app.exit(), and a live pid on a fresh marker would + // make a completed update look active — blocking desktop startup and + // every other updater until the age ceiling expires. + _update_marker.complete(); if let Some(target_app) = launch_target { if let Err(err) = launch_macos_app_and_exit(&app, &target_app).await { @@ -593,9 +611,26 @@ async fn run_update(app: AppHandle) -> Result<()> { ); } + // The launch helpers normally request exit themselves, but their failure + // paths must still close a successful updater. A native event loop can + // ignore that graceful request, so arm a process-exit fallback now that + // all update state and the marker have been settled. + exit_after_success(&app); Ok(()) } +/// Ask the app to exit, with a hard `process::exit` fallback for a native +/// event loop that ignores the graceful request. Without it a finished updater +/// can linger as a live pid forever. +fn exit_after_success(app: &AppHandle) { + std::thread::spawn(|| { + std::thread::sleep(std::time::Duration::from_secs(3)); + tracing::warn!("graceful updater exit timed out; forcing process exit"); + std::process::exit(0); + }); + app.exit(0); +} + /// Poll until the venv shim AND packaged desktop app bundle are no longer locked /// (Windows) or a bounded timeout elapses. On non-Windows this is a short fixed /// grace since file locking isn't the failure mode there. @@ -1327,6 +1362,25 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn completed_update_releases_marker_before_guard_drop() { + let dir = unique_tmp_dir("marker-complete"); + std::fs::create_dir_all(&dir).unwrap(); + let marker = dir.join(".hermes-update-in-progress"); + + let guard = UpdateMarkerGuard::acquire(marker.clone()) + .unwrap_or_else(|_| panic!("no live owner => acquire must succeed")); + guard.complete(); + + assert!( + !marker.exists(), + "a successful update must unblock desktop startup before relaunch/exit" + ); + drop(guard); + assert!(!marker.exists(), "Drop stays idempotent after completion"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn parses_update_branch_from_space_or_equals_args() { assert_eq!(