diff --git a/crates/update/src/updater.rs b/crates/update/src/updater.rs index 5b90061d..0368cb86 100644 --- a/crates/update/src/updater.rs +++ b/crates/update/src/updater.rs @@ -8,8 +8,8 @@ //! [`Updater::managed`] says why. use std::path::PathBuf; -use std::sync::Arc; -use std::sync::mpsc::{Receiver, TryRecvError, channel}; +use std::sync::mpsc::{Receiver, SendError, Sender, TryRecvError, channel}; +use std::sync::{Arc, Mutex, PoisonError}; use std::time::Duration; use crate::dialog::{StartupPrompt, StartupPromptResponse}; @@ -101,6 +101,9 @@ pub struct Updater { restart: Option<(Restart, Release)>, /// An update is installed and waits for the app to exit; nothing starts a second one. installed: bool, + /// Set when the `Updater` is dropped. An install worker sends its outcome while holding it, + /// and starts a finished installation's helper itself once it is set. + dropped: Arc>, /// The version-choice file to read in place of [`Pins::default_path`] (tests). pins_path: Option, /// The launcher's choice for this app when it is not stable, as of the last check or offer. @@ -125,6 +128,7 @@ impl Updater { prompt: None, restart: None, installed: false, + dropped: Arc::new(Mutex::new(false)), pins_path: None, managed: None, translate: english, @@ -400,7 +404,8 @@ impl Updater { /// Starts the helper of an installed update that is still waiting for the app to close, so /// that the update installs once the app has exited. Call it when the app exits by a path /// that did not come through [`Action::Close`]; dropping the `Updater` calls it too. Later - /// calls do nothing. An installation still running is left alone. + /// calls do nothing. An installation still running when the `Updater` is dropped starts its + /// helper itself when it finishes. pub fn start_pending_update(&mut self) { // An installation that finished since the last `tick`. if let Some((rx, release)) = &self.install @@ -452,10 +457,11 @@ impl Updater { let (tx, rx) = channel(); let (app, version, ctx, release) = (self.app, self.version.clone(), ctx.clone(), prompt.release.clone()); let worker_release = release.clone(); + let dropped = Arc::clone(&self.dropped); let spawned = std::thread::Builder::new().name("self-update".into()).spawn(move || { let run = || install(app, &installed, worker_release, &version).map_err(Failure::Installer); let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(run)).unwrap_or_else(|_| Err(Failure::panicked())); - let _ = tx.send(outcome); + deliver(app, &tx, &dropped, outcome); ctx.request_repaint(); }); match spawned { @@ -472,10 +478,29 @@ impl Drop for Updater { /// The app is exiting: an installed update that is still waiting starts its helper, which /// waits for this process to exit before it installs. fn drop(&mut self) { + // Held while the channel is read: a worker sent its outcome before, and it is read here, + // or it sends after and starts its helper itself. + let dropped = Arc::clone(&self.dropped); + let mut dropped = dropped.lock().unwrap_or_else(PoisonError::into_inner); + *dropped = true; self.start_pending_update(); } } +/// Sends the install worker's outcome to the [`Updater`]. When the `Updater` has been dropped +/// (the app is exiting), the worker starts a finished installation's helper itself, since +/// nothing will start it at a close. +fn deliver(app: &App, tx: &Sender>, dropped: &Mutex, outcome: Result) { + let gone = dropped.lock().unwrap_or_else(PoisonError::into_inner); + let unsent = if *gone { Some(outcome) } else { tx.send(outcome).err().map(|SendError(outcome)| outcome) }; + drop(gone); + if let Some(Ok(restart)) = unsent + && let Err(e) = restart.start() + { + log::warn!("{} self-update failed: {e}", app.name); + } +} + fn install(app: &'static App, installed: &self_update::Install, mut release: Release, version: &str) -> Result { let http = Http::new(&format!("{}/Update", app.name)).map_err(|e| format!("could not initialize HTTP: {e}"))?; if release.assets.is_empty() @@ -905,6 +930,34 @@ mod tests { assert_eq!(starts.load(Ordering::SeqCst), 1, "the helper starts once"); } + /// An installation that finishes after the updater was dropped starts its helper once; one + /// the updater receives waits for the close. + #[test] + fn an_install_finishing_after_the_updater_is_dropped_starts_its_helper() { + use std::sync::atomic::Ordering; + let mut u = updater(Ok(None)); + let dropped = Arc::clone(&u.dropped); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 0); + deliver(&crate::suite::LAUNCHER, &tx, &dropped, Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 1); + + // Sent while the drop has read the channel but not yet released the receiver. + let (starts, restart) = counted_restart(); + let (tx, _rx) = channel(); + deliver(&crate::suite::LAUNCHER, &tx, &Mutex::new(true), Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 1, "a receiver about to be dropped is not sent to"); + + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + deliver(&crate::suite::LAUNCHER, &tx, &Mutex::new(false), Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 0, "the updater starts it at the close"); + assert!(rx.try_recv().is_ok_and(|outcome| outcome.is_ok())); + } + /// A failed installation picked up at exit leaves nothing installed and nothing waiting. #[test] fn a_failed_install_picked_up_at_exit_leaves_nothing_waiting() {