diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 67643ee634..4193e8185c 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -194,35 +194,53 @@ struct MacOsUnmounter<'a> { background_executor: &'a BackgroundExecutor, } +impl MacOsUnmounter<'_> { + /// Unmounts the disk image and waits for completion. This must happen + /// before the `InstallerDir` is dropped: deleting the temp dir while the + /// image is still mounted inside it fails silently and leaks the + /// directory (and the downloaded DMG) in the system temp dir. + async fn unmount(mut self) { + let mount_path = mem::take(&mut self.mount_path); + unmount_disk_image(&mount_path).await; + } +} + impl Drop for MacOsUnmounter<'_> { fn drop(&mut self) { let mount_path = mem::take(&mut self.mount_path); + // Safety net for early exits and cancellation; the happy path calls + // `unmount`, which leaves the path empty. + if mount_path.as_os_str().is_empty() { + return; + } self.background_executor - .spawn(async move { - let unmount_output = new_command("hdiutil") - .args(["detach", "-force"]) - .arg(&mount_path) - .output() - .await; - match unmount_output { - Ok(output) if output.status.success() => { - log::info!("Successfully unmounted the disk image"); - } - Ok(output) => { - log::error!( - "Failed to unmount disk image: {:?}", - String::from_utf8_lossy(&output.stderr) - ); - } - Err(error) => { - log::error!("Error while trying to unmount disk image: {:?}", error); - } - } - }) + .spawn(async move { unmount_disk_image(&mount_path).await }) .detach(); } } +async fn unmount_disk_image(mount_path: &Path) { + let unmount_output = new_command("hdiutil") + .args(["detach", "-force"]) + .arg(mount_path) + .output() + .await; + match unmount_output { + Ok(output) if output.status.success() => { + log::info!("Successfully unmounted the disk image"); + } + Ok(output) => { + log::error!( + "Failed to unmount disk image: {:?}", + String::from_utf8_lossy(&output.stderr) + ); + } + Err(error) => { + log::error!("Error while trying to unmount disk image: {:?}", error); + } + } +} + #[derive(Clone, Copy, Debug, RegisterSetting)] struct AutoUpdateSetting(bool); @@ -345,6 +363,9 @@ pub fn view_release_notes(_: &ViewReleaseNotes, cx: &mut App) -> Option<()> { None } +#[cfg(not(target_os = "windows"))] +const INSTALLER_DIR_PREFIX: &str = "zed-auto-update"; + #[cfg(not(target_os = "windows"))] struct InstallerDir(tempfile::TempDir); @@ -353,7 +374,7 @@ impl InstallerDir { async fn new() -> Result { Ok(Self( tempfile::Builder::new() - .prefix("zed-auto-update") + .prefix(INSTALLER_DIR_PREFIX) .tempdir()?, )) } @@ -448,6 +469,9 @@ impl AutoUpdater { .log_err(); } + #[cfg(all(not(target_os = "windows"), not(test)))] + cx.background_spawn(cleanup_stale_installer_dirs()).detach(); + loop { this.update(cx, |this, cx| this.poll(UpdateCheckType::Automatic, cx))?; cx.background_executor().timer(poll_interval).await; @@ -1161,8 +1185,7 @@ async fn install_release_macos( String::from_utf8_lossy(&output.stderr) ); - // Create an MacOsUnmounter that will be dropped (and thus unmount the disk) when this function exits - let _unmounter = MacOsUnmounter { + let unmounter = MacOsUnmounter { mount_path: mount_path.clone(), background_executor, }; @@ -1171,10 +1194,13 @@ async fn install_release_macos( cmd.args(["-av", "--delete", "--exclude", "Icon?"]) .arg(&mounted_app_path) .arg(&running_app_path); - let output = cmd - .output() - .await - .with_context(|| "failed to rsync: {cmd}")?; + let rsync_output = cmd.output().await; + + // Await the unmount (even if rsync failed) so that the installer temp dir + // can be deleted once this function returns. + unmounter.unmount().await; + + let output = rsync_output.with_context(|| "failed to rsync: {cmd}")?; anyhow::ensure!( output.status.success(), @@ -1185,6 +1211,52 @@ async fn install_release_macos( Ok(None) } +/// Removes stale installer dirs from the system temp dir. Older Zed versions +/// leaked one per update by deleting the dir while the downloaded disk image +/// was still mounted inside it, which made the deletion fail silently. +#[cfg(any(rust_analyzer, all(not(target_os = "windows"), not(test))))] +async fn cleanup_stale_installer_dirs() { + const STALE_INSTALLER_DIR_AGE: Duration = Duration::from_secs(24 * 60 * 60); + + let temp_dir = std::env::temp_dir(); + let Ok(mut entries) = fs::read_dir(&temp_dir).await else { + log::warn!("failed to read temp dir {temp_dir:?} while cleaning up installer dirs"); + return; + }; + while let Some(entry) = entries.next().await { + let Ok(entry) = entry else { + continue; + }; + if !entry + .file_name() + .to_string_lossy() + .starts_with(INSTALLER_DIR_PREFIX) + { + continue; + } + // Leave recent dirs alone, as they may belong to an update currently + // in progress in another Zed instance. + let is_stale = entry.metadata().await.ok().is_some_and(|metadata| { + metadata.is_dir() + && metadata.modified().ok().is_some_and(|modified| { + SystemTime::now() + .duration_since(modified) + .is_ok_and(|age| age > STALE_INSTALLER_DIR_AGE) + }) + }); + if is_stale { + if let Err(error) = fs::remove_dir_all(entry.path()).await { + log::warn!( + "failed to remove stale installer dir {:?}: {error}", + entry.path() + ); + } else { + log::info!("removed stale installer dir {:?}", entry.path()); + } + } + } +} + async fn cleanup_windows() -> Result<()> { let parent = std::env::current_exe()? .parent()