craft-update: start the Windows update helper when the app closes, and never restart a running app

On Windows the update helper was started as soon as the download was
ready, then waited one minute for the app to exit. An app that held
unsaved work, or a user who did not close it, outlasted that wait; the
helper then started the old exe again with no arguments, which opened a
second window of the old version and installed nothing.

- self_update::prepare_update does what update did but returns a
  Restart. On Windows the Restart records the release as installed and
  starts the helper; on macOS and Linux it does nothing, since the
  restart is already scheduled. update calls prepare_update and starts
  the Restart at once, so its callers see no change.
- Updater starts the Restart when it returns Action::Close. If the
  helper can't start, the app stays open and the prompt shows the error.
- The helper does not start any exe when the app is still running after
  its wait. A step that fails after the app exited still starts the old
  exe, as before.

crates/update/src/self_update.rs

@@ -244,11 +244,12 @@ fn msi_step(app: &App, exe: &Path) -> Result<crate::msi_migrate::Step, String> {
     crate::msi_migrate::windows::step(app, exe)
 }
 
-/// Whether the helper starts the app after the update step: always after a successful one, and
-/// after a failed one only while its exe is still there.
+/// Whether the helper starts the app after the update step: never while the old app is still
+/// running (a second copy of it would open), always after a successful step, and after a failed
+/// one only while its exe is still there.
 #[cfg_attr(not(windows), allow(dead_code))]
-fn restarts(step_succeeded: bool, exe_exists: bool) -> bool {
-    step_succeeded || exe_exists
+fn restarts(app_exited: bool, step_succeeded: bool, exe_exists: bool) -> bool {
+    app_exited && (step_succeeded || exe_exists)
 }
 
 /// The exe to start after an MSI update of a registered copy: the one its registrations name now
@@ -371,6 +372,58 @@ pub fn update(
     last: &dyn Fn() -> Result<(), String>,
     running_version: Option<&str>,
 ) -> Result<(), String> {
+    prepare_update(app, install, release, platform, http, paths, stop, phase, last, running_version)?.start()
+}
+
+/// What remains of an update after [`prepare_update`]: starting the Windows helper, which waits
+/// for this process to exit and then installs the update. On macOS and Linux the update is
+/// already in place and its restart already scheduled, so [`Restart::start`] does nothing.
+#[must_use = "on Windows the update is only installed once the restart is started"]
+pub struct Restart {
+    helper: Option<Box<dyn FnOnce() -> Result<(), String> + Send>>,
+}
+
+impl Restart {
+    /// Nothing is left to start.
+    #[cfg_attr(all(windows, not(test)), allow(dead_code))]
+    pub(crate) fn scheduled() -> Self {
+        Self { helper: None }
+    }
+
+    /// `start` runs when the app closes.
+    #[cfg_attr(not(any(windows, test)), allow(dead_code))]
+    pub(crate) fn later(start: impl FnOnce() -> Result<(), String> + Send + 'static) -> Self {
+        Self { helper: Some(Box::new(start)) }
+    }
+
+    /// Starts the helper. Call it when the app closes: the helper waits only a minute for the
+    /// app to exit, and installs nothing when the app is still running after that.
+    pub fn start(self) -> Result<(), String> {
+        self.helper.map_or(Ok(()), |start| start())
+    }
+}
+
+impl std::fmt::Debug for Restart {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Restart").field("pending", &self.helper.is_some()).finish()
+    }
+}
+
+/// [`update`] without starting the Windows helper: the caller starts the returned [`Restart`]
+/// when the app closes. On macOS and Linux the result is the same as [`update`]'s.
+#[allow(clippy::too_many_arguments)]
+pub fn prepare_update(
+    app: &App,
+    install: &Install,
+    release: &Release,
+    platform: Platform,
+    http: &Http,
+    paths: &Paths,
+    stop: &Stop,
+    phase: &mut dyn FnMut(JobPhase),
+    last: &dyn Fn() -> Result<(), String>,
+    running_version: Option<&str>,
+) -> Result<Restart, String> {
     let cur_v = running_version.and_then(|v| current_version(app, v));
     if let (Some(rel_v), Some(cur_v)) = (release.version.as_ref(), cur_v.as_ref())
         && !version::offers_update(rel_v, cur_v)
@@ -390,11 +443,13 @@ pub fn update(
     // The restart ends this process: nothing else may be under way.
     last()?;
     phase(JobPhase::Installing);
-    // Written before the swap starts: the Windows helper runs alongside this process and clears
-    // it when it fails, which a write after scheduling could undo.
+    // Written before the swap starts. On Windows the helper does the swap, so the record is
+    // written when it starts (see `windows_helper`): it clears the record when it fails, which a
+    // write after it started could undo.
+    #[cfg(not(windows))]
     mark_installed(paths, release);
     let pid = std::process::id();
-    let scheduled = (|| -> Result<(), String> {
+    let scheduled = (|| -> Result<Restart, String> {
         match install {
             #[cfg(target_os = "macos")]
             Install::MacApp(bundle) => {
@@ -402,20 +457,20 @@ pub fn update(
                 let placed = install::macos::install_dmg(app, &file, Some(&existing), None, false, paths, &install::macos::Expect::release(release));
                 let _ = std::fs::remove_file(&file);
                 let placed = placed?;
-                relaunch_after_exit(app, pid, &[Path::new("/usr/bin/open").as_os_str(), placed.as_os_str()])
+                relaunch_after_exit(app, pid, &[Path::new("/usr/bin/open").as_os_str(), placed.as_os_str()]).map(|()| Restart::scheduled())
             }
             #[cfg(target_os = "linux")]
             Install::AppImage(path) => {
                 replace_appimage(&file, path)?;
                 let _ = std::fs::remove_file(&file);
-                relaunch_after_exit(app, pid, &[path.as_os_str()])
+                relaunch_after_exit(app, pid, &[path.as_os_str()]).map(|()| Restart::scheduled())
             }
             #[cfg(target_os = "linux")]
             Install::Package(kind, exe) => {
                 let result = install::linux::install_package(*kind, &file, None, stop, phase);
                 let _ = std::fs::remove_file(&file);
                 result?;
-                relaunch_after_exit(app, pid, &[exe.as_os_str()])
+                relaunch_after_exit(app, pid, &[exe.as_os_str()]).map(|()| Restart::scheduled())
             }
             #[cfg(target_os = "linux")]
             Install::Flatpak => {
@@ -429,26 +484,26 @@ pub fn update(
                 let result = install::linux::update_flatpak_bundle(&file, &app.bundle_id(), system, stop, phase);
                 let _ = std::fs::remove_file(&file);
                 result?;
-                relaunch_flatpak_after_exit(app)
+                relaunch_flatpak_after_exit(app).map(|()| Restart::scheduled())
             }
             #[cfg(windows)]
-            Install::Msi(exe) => windows_helper(app, paths, pid, exe, &msi_finish(&file)?),
+            Install::Msi(exe) => windows_helper(app, paths, release, pid, exe, &msi_finish(&file)?),
             #[cfg(windows)]
             Install::Portable(exe) => {
                 exe.parent().ok_or_else(|| format!("{} isn't in a folder", app.name))?;
-                windows_helper(app, paths, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) })
+                windows_helper(app, paths, release, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) })
             }
             #[cfg(windows)]
             Install::Standalone(exe) => {
                 exe.parent().ok_or_else(|| format!("{} isn't in a folder", app.name))?;
-                windows_helper(app, paths, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) })
+                windows_helper(app, paths, release, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) })
             }
             #[cfg(unix)]
             Install::Standalone(exe) => {
                 let result = replace_standalone(app, paths, &file, exe);
                 let _ = std::fs::remove_file(&file);
                 result?;
-                relaunch_after_exit(app, pid, &[exe.as_os_str()])
+                relaunch_after_exit(app, pid, &[exe.as_os_str()]).map(|()| Restart::scheduled())
             }
             #[allow(unreachable_patterns)]
             _ => {
@@ -469,7 +524,7 @@ fn mark_installed(paths: &Paths, release: &Release) {
 
 /// Removes the record of [`mark_installed`] when the swap couldn't be scheduled, so the update
 /// stays on offer.
-fn unmark_unless_scheduled(paths: &Paths, scheduled: Result<(), String>) -> Result<(), String> {
+fn unmark_unless_scheduled<T>(paths: &Paths, scheduled: Result<T, String>) -> Result<T, String> {
     if scheduled.is_err() {
         let _ = std::fs::remove_file(marker(paths));
     }
@@ -759,11 +814,12 @@ const HELPER_DIR: &str = "launcher-update-helper";
 #[cfg_attr(not(windows), allow(dead_code))]
 const PORTABLE_STAGING: &str = "launcher-update";
 
-/// Copies this program into the cache and starts the copy as the update helper, which waits for
-/// `pid` to exit, does `finish` and starts `exe`. Run from the cache, the helper doesn't hold the
-/// installed exe that the step replaces.
+/// Copies this program into the cache as the update helper, which waits for `pid` to exit, does
+/// `finish` and starts `exe`. Run from the cache, the helper doesn't hold the installed exe that
+/// the step replaces. The returned [`Restart`] records `release` as installed and starts the
+/// helper.
 #[cfg(windows)]
-fn windows_helper(app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finish) -> Result<(), String> {
+fn windows_helper(app: &App, paths: &Paths, release: &Release, pid: u32, exe: &Path, finish: &Finish) -> Result<Restart, String> {
     let program = std::env::current_exe().map_err(|e| format!("couldn't find this program ({e})"))?;
     let dir = paths.cache.join(HELPER_DIR);
     // The previous update's copy, when it's still there.
@@ -774,7 +830,12 @@ fn windows_helper(app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finis
     let mut cmd = process::command(&helper);
     cmd.args(helper_args(pid, exe, finish));
     clear_pretend_version(&mut cmd, app);
-    process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the update ({e})"))
+    let (paths, release) = (paths.clone(), release.clone());
+    Ok(Restart::later(move || {
+        mark_installed(&paths, &release);
+        let started = process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the update ({e})"));
+        unmark_unless_scheduled(&paths, started)
+    }))
 }
 
 /// Finishes an update when this program was started as the update helper; `args` are its
@@ -799,23 +860,48 @@ pub fn finish(app: &App, args: &[OsString]) -> Option<Result<(), String>> {
     // Read before the step: the update may move or replace the old exe.
     let registered =
         matches!(step, Finish::Msi(_)) && matches!(crate::msi_copy::windows::find(app, &exe), Some(crate::msi_copy::MsiCopy::Registered { .. }));
-    let done = run_step(app, &me, pid, &exe, &step, std::time::Duration::from_secs(60));
+    let wait = |pid| process::wait_for_exit(pid, std::time::Duration::from_secs(60));
+    let start = |exe: &Path| {
+        signed_like(app, &me, exe)
+            .and_then(|()| process::spawn_detached(std::process::Command::new(exe)).map_err(|e| format!("couldn't start {} ({e})", exe.display())))
+    };
+    Some(finish_step(app, &me, &paths, pid, exe, &step, registered, &wait, &start))
+}
+
+/// The helper's work once its checks have passed: waits for `pid` with `wait`, does `step`, and
+/// starts the app with `start` when [`restarts`] allows it. Every failure is recorded for the
+/// app's next start.
+#[cfg(windows)]
+#[allow(clippy::too_many_arguments)]
+fn finish_step(
+    app: &App,
+    reference: &Path,
+    paths: &Paths,
+    pid: u32,
+    exe: PathBuf,
+    step: &Finish,
+    registered: bool,
+    wait: &dyn Fn(u32) -> bool,
+    start: &dyn Fn(&Path) -> Result<(), String>,
+) -> Result<(), String> {
+    let stepped = step_after_wait(app, reference, pid, &exe, step, wait);
+    let done = stepped.result;
     if let Err(e) = &done {
-        record_failure(&paths, e);
+        record_failure(paths, e);
     }
     // The new app, or the old one when the step failed.
     let exe = if done.is_ok() { exe_after_msi(app, registered, &exe) } else { exe };
-    // A failed move can leave no program to start; its recorded error already says how to get
-    // the app back, and a restart error must not replace it.
-    if !restarts(done.is_ok(), exe.is_file()) {
-        return Some(done);
+    // An app that is still running gets no second copy. A failed move can leave no program to
+    // start; its recorded error already says how to get the app back, and a restart error must
+    // not replace it.
+    if !restarts(stepped.exited, done.is_ok(), exe.is_file()) {
+        return done;
     }
-    let started = signed_like(app, &me, &exe)
-        .and_then(|()| process::spawn_detached(std::process::Command::new(&exe)).map_err(|e| format!("couldn't start {} ({e})", exe.display())));
+    let started = start(&exe);
     if let Err(e) = &started {
-        record_failure(&paths, e);
+        record_failure(paths, e);
     }
-    Some(done.and(started))
+    done.and(started)
 }
 
 /// Finishes an update on non-Windows platforms (no-op helper).
@@ -990,14 +1076,29 @@ mod wait_watch {
     }
 }
 
-/// Waits up to `wait` for `pid` to exit, does `step` once it has, and removes the download either
-/// way. The MSI, and every program in the portable zip, must be signed like `reference`.
-#[cfg(windows)]
+/// [`step_after_wait`], waiting up to `wait` for `pid` to exit.
+#[cfg(all(windows, test))]
 fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: std::time::Duration) -> Result<(), String> {
+    step_after_wait(app, reference, pid, exe, step, &|pid| process::wait_for_exit(pid, wait)).result
+}
+
+/// What [`step_after_wait`] came to.
+#[cfg(windows)]
+struct Stepped {
+    /// The old app had exited when the wait returned.
+    exited: bool,
+    result: Result<(), String>,
+}
+
+/// Calls `wait`, which says whether `pid` exited in time, does `step` once it has, and removes
+/// the download either way. The MSI, and every program in the portable zip, must be signed like
+/// `reference`.
+#[cfg(windows)]
+fn step_after_wait(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: &dyn Fn(u32) -> bool) -> Stepped {
     #[cfg(test)]
     wait_watch::entered(pid);
     // The test hook needs the result, so it is stored rather than consumed by the branch.
-    let waited = process::wait_for_exit(pid, wait);
+    let waited = wait(pid);
     #[cfg(test)]
     wait_watch::returned(pid, waited);
     let result = if waited {
@@ -1025,7 +1126,7 @@ fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wa
             let _ = std::fs::remove_file(zip);
         }
     }
-    result
+    Stepped { exited: waited, result }
 }
 
 #[cfg(test)]
@@ -1434,7 +1535,7 @@ mod tests {
         std::fs::create_dir_all(&paths.data).unwrap();
         let release = Release { tag: "v0.2.0".into(), ..Release::default() };
         mark_installed(&paths, &release);
-        assert!(unmark_unless_scheduled(&paths, Err("couldn't schedule the update".into())).is_err());
+        assert!(unmark_unless_scheduled(&paths, Err::<(), _>("couldn't schedule the update".into())).is_err());
         assert!(!just_updated_to(&paths, &release));
         mark_installed(&paths, &release);
         assert!(unmark_unless_scheduled(&paths, Ok(())).is_ok());
@@ -1986,10 +2087,118 @@ mod tests {
 
     #[test]
     fn a_failed_step_that_removed_the_app_starts_nothing_and_keeps_its_message() {
-        assert!(restarts(true, true));
-        assert!(restarts(true, false));
-        assert!(restarts(false, true));
-        assert!(!restarts(false, false));
+        assert!(restarts(true, true, true));
+        assert!(restarts(true, true, false));
+        assert!(restarts(true, false, true));
+        assert!(!restarts(true, false, false));
+    }
+
+    /// An app still running after the helper's wait never gets a second copy, whatever the step
+    /// and the exe would allow.
+    #[test]
+    fn an_app_still_running_is_never_started_again() {
+        for (step_succeeded, exe_exists) in [(true, true), (true, false), (false, true), (false, false)] {
+            assert!(!restarts(false, step_succeeded, exe_exists), "{step_succeeded} {exe_exists}");
+        }
+    }
+
+    /// A helper whose wait ends with the old app still running installs nothing, starts no
+    /// program, records why, and removes the download.
+    #[cfg(windows)]
+    #[test]
+    fn the_helper_starts_nothing_while_the_old_app_runs() {
+        let dir = tempfile::tempdir().unwrap();
+        let paths = Paths::under(dir.path());
+        let exe = dir.path().join("artcraft-launcher.exe");
+        std::fs::write(&exe, "0.1.0").unwrap();
+        let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip");
+        std::fs::write(&zip, "zip").unwrap();
+        let step = Finish::Portable { zip: zip.clone(), tmp: dir.path().join(PORTABLE_STAGING) };
+        let started = std::sync::Mutex::new(Vec::<PathBuf>::new());
+        let start = |exe: &Path| {
+            started.lock().unwrap().push(exe.to_path_buf());
+            Ok(())
+        };
+        let never_exits = |_pid| false;
+        let error = finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &never_exits, &start)
+            .unwrap_err();
+        assert!(error.contains("still running"), "{error}");
+        assert_eq!(*started.lock().unwrap(), Vec::<PathBuf>::new(), "no copy of the app may start while it still runs");
+        assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0");
+        assert!(!zip.exists(), "the download is removed");
+        assert_eq!(take_failure(&paths).as_deref(), Some(error.as_str()));
+    }
+
+    /// A step that fails after the old app exited starts the old app again, as it was.
+    #[cfg(windows)]
+    #[test]
+    fn the_helper_starts_the_old_app_after_a_failed_step() {
+        let dir = tempfile::tempdir().unwrap();
+        let paths = Paths::under(dir.path());
+        let exe = dir.path().join("artcraft-launcher.exe");
+        std::fs::write(&exe, "0.1.0").unwrap();
+        // Not a zip, so the step fails.
+        let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip");
+        std::fs::write(&zip, "not a zip").unwrap();
+        let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) };
+        let started = std::sync::Mutex::new(Vec::<PathBuf>::new());
+        let start = |exe: &Path| {
+            started.lock().unwrap().push(exe.to_path_buf());
+            Ok(())
+        };
+        let exited = |_pid| true;
+        assert!(finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start).is_err());
+        assert_eq!(*started.lock().unwrap(), vec![exe.clone()]);
+        assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0");
+    }
+
+    /// A successful step starts the new app once.
+    #[cfg(windows)]
+    #[test]
+    fn the_helper_starts_the_new_app_after_the_update() {
+        use std::io::Write;
+        let dir = tempfile::tempdir().unwrap();
+        let paths = Paths::under(dir.path());
+        let folder = dir.path().join("ArtCraft Launcher");
+        std::fs::create_dir_all(&folder).unwrap();
+        let exe = folder.join("artcraft-launcher.exe");
+        std::fs::write(&exe, "0.1.0").unwrap();
+        let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip");
+        let mut writer = zip::ZipWriter::new(std::fs::File::create(&zip).unwrap());
+        let signed_exe = std::fs::read(system_program("cmd.exe")).unwrap();
+        writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/artcraft-launcher.exe", zip::write::SimpleFileOptions::default()).unwrap();
+        writer.write_all(&signed_exe).unwrap();
+        writer.finish().unwrap();
+        let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) };
+        let started = std::sync::Mutex::new(Vec::<(PathBuf, Vec<u8>)>::new());
+        let start = |exe: &Path| {
+            started.lock().unwrap().push((exe.to_path_buf(), std::fs::read(exe).unwrap()));
+            Ok(())
+        };
+        let exited = |_pid| true;
+        assert_eq!(
+            finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start),
+            Ok(())
+        );
+        assert_eq!(*started.lock().unwrap(), vec![(exe, signed_exe)], "the new app starts once");
+        assert_eq!(take_failure(&paths), None);
+    }
+
+    /// A prepared Windows restart does nothing until it is started, and runs once when it is.
+    #[test]
+    fn a_prepared_restart_runs_only_when_started() {
+        use std::sync::Arc;
+        use std::sync::atomic::{AtomicUsize, Ordering};
+        let runs = Arc::new(AtomicUsize::new(0));
+        let counter = Arc::clone(&runs);
+        let restart = Restart::later(move || {
+            counter.fetch_add(1, Ordering::SeqCst);
+            Ok(())
+        });
+        assert_eq!(runs.load(Ordering::SeqCst), 0, "nothing starts before the app closes");
+        assert_eq!(restart.start(), Ok(()));
+        assert_eq!(runs.load(Ordering::SeqCst), 1);
+        assert_eq!(Restart::scheduled().start(), Ok(()));
     }
 
     #[test]

crates/update/src/updater.rs

@@ -15,6 +15,7 @@ use std::time::Duration;
 use crate::dialog::{StartupPrompt, StartupPromptResponse};
 use crate::pins::{Choice, Pins};
 use crate::release::Release;
+use crate::self_update::Restart;
 use crate::translate::{Note, Translate, english};
 use crate::{App, Http, Paths, Platform, Stop, check_for_update, self_update};
 
@@ -42,7 +43,8 @@ pub enum Action {
     /// Open this page in the browser.
     OpenUrl(String),
     /// Close the app through its normal close path (which asks about unsaved work): the update is
-    /// installed and a helper restarts the app once it has exited.
+    /// installed and a helper restarts the app once it has exited. On Windows the helper, which
+    /// installs the update, starts when this is returned.
     Close,
 }
 
@@ -89,9 +91,11 @@ pub struct Updater {
     requested: bool,
     started: bool,
     check: Option<Receiver<Checked>>,
-    install: Option<(Receiver<Result<(), Failure>>, Release)>,
+    install: Option<(Receiver<Result<Restart, Failure>>, Release)>,
     prompt: Option<StartupPrompt>,
-    restart: bool,
+    /// A finished installation waiting for the app to close: what still has to start then, and
+    /// the release, to offer again if it can't.
+    restart: Option<(Restart, Release)>,
     /// An update is installed and waits for the app to exit; nothing starts a second one.
     installed: bool,
     /// The version-choice file to read in place of [`Pins::default_path`] (tests).
@@ -116,7 +120,7 @@ impl Updater {
             check: None,
             install: None,
             prompt: None,
-            restart: false,
+            restart: None,
             installed: false,
             pins_path: None,
             managed: None,
@@ -308,9 +312,9 @@ impl Updater {
             if let Some(outcome) = outcome {
                 let release = self.install.take().map(|(_, release)| release);
                 match outcome {
-                    Ok(()) => {
+                    Ok(restart) => {
                         self.installed = true;
-                        self.restart = true;
+                        self.restart = release.map(|release| (restart, release));
                         let note = self.installed_note();
                         if let Some(prompt) = &mut self.prompt {
                             prompt.is_installing = false;
@@ -351,10 +355,24 @@ impl Updater {
     /// and a close that the user then cancels would leave it waiting for an exit that never comes.
     pub fn show_if(&mut self, ctx: &egui::Context, can_start: bool) -> Action {
         // The close waits for the app to hold no unsaved work: the user may have edited while the
-        // update installed, and a close they cancel is asked again once the work is saved.
-        if self.restart && can_start {
-            self.restart = false;
-            return Action::Close;
+        // update installed. The Windows helper starts only now, since it waits just a minute for
+        // the app to exit.
+        if can_start && let Some((restart, release)) = self.restart.take() {
+            match restart.start() {
+                Ok(()) => return Action::Close,
+                Err(e) => {
+                    log::warn!("{} self-update failed: {e}", self.app.name);
+                    // Nothing will install it: the update is on offer again.
+                    self.installed = false;
+                    if self.prompt.is_none() {
+                        self.prompt = Some(self.prompt_for(release));
+                    }
+                    if let Some(prompt) = &mut self.prompt {
+                        prompt.is_installing = false;
+                        prompt.set_message(e);
+                    }
+                }
+            }
         }
         let Some(prompt) = &mut self.prompt else { return Action::None };
         match prompt.show(ctx) {
@@ -421,7 +439,7 @@ impl Updater {
     }
 }
 
-fn install(app: &'static App, installed: &self_update::Install, mut release: Release, version: &str) -> Result<(), String> {
+fn install(app: &'static App, installed: &self_update::Install, mut release: Release, version: &str) -> Result<Restart, String> {
     let http = Http::new(&format!("{}/Update", app.name)).map_err(|e| format!("could not initialize HTTP: {e}"))?;
     if release.assets.is_empty()
         && let Some(full) = check_for_update(app, &http, version).map_err(|e| format!("could not fetch release assets: {e}"))?
@@ -430,7 +448,7 @@ fn install(app: &'static App, installed: &self_update::Install, mut release: Rel
     }
     let paths = Paths::for_app(app).map_err(|e| format!("could not find paths: {e}"))?;
     let platform = Platform::current().ok_or("unsupported platform for self update")?;
-    self_update::update(app, installed, &release, platform, &http, &paths, &Stop::default(), &mut |_| {}, &|| Ok(()), Some(version))
+    self_update::prepare_update(app, installed, &release, platform, &http, &paths, &Stop::default(), &mut |_| {}, &|| Ok(()), Some(version))
 }
 
 /// Download in the prompt: the release page, when it is a safe web link (an app may build its
@@ -642,7 +660,7 @@ mod tests {
     fn a_dropped_install_worker_is_an_error() {
         let ctx = egui::Context::default();
         let mut u = updater(Ok(None));
-        let (tx, rx) = channel::<Result<(), Failure>>();
+        let (tx, rx) = channel::<Result<Restart, Failure>>();
         u.install = Some((rx, newer()));
         drop(tx);
         u.tick(&ctx);
@@ -701,7 +719,7 @@ mod tests {
         let mut u = updater(Ok(Some(newer())));
         let (tx, rx) = channel();
         u.install = Some((rx, newer()));
-        assert!(tx.send(Ok(())).is_ok());
+        assert!(tx.send(Ok(Restart::scheduled())).is_ok());
         u.tick(&ctx);
         assert_eq!(u.show(&ctx), Action::Close);
         u.prompt = None;
@@ -742,7 +760,7 @@ mod tests {
         // An installed update names the app, in the language of the moment.
         let (tx, rx) = channel();
         u.install = Some((rx, newer()));
-        assert!(tx.send(Ok(())).is_ok());
+        assert!(tx.send(Ok(Restart::scheduled())).is_ok());
         u.tick(&ctx);
         assert!(message(&u).starts_with("FR The update is installed. Close PhotoCraft"), "{}", message(&u));
         FRENCH.store(false, Ordering::Relaxed);
@@ -763,7 +781,7 @@ mod tests {
         u.offer(newer());
         let (tx, rx) = channel();
         u.install = Some((rx, newer()));
-        assert!(tx.send(Ok(())).is_ok());
+        assert!(tx.send(Ok(Restart::scheduled())).is_ok());
         u.tick(&ctx);
         assert_eq!(frame(&mut u, &ctx, false), Action::None, "unsaved work holds the close back");
         assert_eq!(frame(&mut u, &ctx, true), Action::Close);
@@ -776,11 +794,53 @@ mod tests {
         let mut u = updater(Ok(None));
         let (tx, rx) = channel();
         u.install = Some((rx, newer()));
-        assert!(tx.send(Ok(())).is_ok());
+        assert!(tx.send(Ok(Restart::scheduled())).is_ok());
         u.tick(&ctx);
         assert_eq!(u.show(&ctx), Action::Close);
         assert_eq!(u.show(&ctx), Action::None);
     }
+
+    /// A finished installation that still has to start its helper starts it only when the app
+    /// may close, and only once.
+    #[test]
+    fn the_helper_starts_once_when_the_app_closes() {
+        use std::sync::atomic::{AtomicUsize, Ordering};
+        let ctx = egui::Context::default();
+        let mut u = updater(Ok(None));
+        u.offer(newer());
+        let starts = Arc::new(AtomicUsize::new(0));
+        let counter = Arc::clone(&starts);
+        let (tx, rx) = channel();
+        u.install = Some((rx, newer()));
+        let restart = Restart::later(move || {
+            counter.fetch_add(1, Ordering::SeqCst);
+            Ok(())
+        });
+        assert!(tx.send(Ok(restart)).is_ok());
+        u.tick(&ctx);
+        assert_eq!(starts.load(Ordering::SeqCst), 0, "the helper waits for the close");
+        assert_eq!(frame(&mut u, &ctx, false), Action::None);
+        assert_eq!(starts.load(Ordering::SeqCst), 0, "unsaved work holds the helper back");
+        assert_eq!(frame(&mut u, &ctx, true), Action::Close);
+        assert_eq!(starts.load(Ordering::SeqCst), 1);
+        assert_eq!(frame(&mut u, &ctx, true), Action::None);
+        assert_eq!(starts.load(Ordering::SeqCst), 1, "the helper starts once");
+    }
+
+    /// A helper that can't start keeps the app open and offers the update again with the reason.
+    #[test]
+    fn a_helper_that_cannot_start_keeps_the_app_open() {
+        let ctx = egui::Context::default();
+        let mut u = updater(Ok(None));
+        let (tx, rx) = channel();
+        u.install = Some((rx, newer()));
+        assert!(tx.send(Ok(Restart::later(|| Err("couldn't schedule the update (denied)".into())))).is_ok());
+        u.tick(&ctx);
+        assert_eq!(frame(&mut u, &ctx, true), Action::None);
+        let prompt = u.prompt.as_ref().map(|p| (p.is_installing, p.status_message.clone()));
+        assert_eq!(prompt, Some((false, Some("couldn't schedule the update (denied)".into()))));
+        assert!(!u.installed, "the update is on offer again");
+    }
 }
 
 #[cfg(test)]