auto_update: Fix installer temp dirs leaking on macOS (#60528)
The unmount of the update disk image was made async-and-detached in #38867, which introduced a race: the installer TempDir was dropped (running remove_dir_all) while the DMG was still mounted inside it. The removal failed silently, leaking a zed-auto-update* dir containing the ~140 MB DMG in /private/var/folders on every update. Now the unmount is awaited before the temp dir is dropped (installation already runs on the background executor since #58767, so this no longer blocks the UI), with the Drop impl kept as a safety net for early exits and cancellation. Additionally, stale installer dirs older than 24 hours are swept from the temp dir when update polling starts, so existing accumulated leaks get reclaimed. Closes FR-104 Closes #58835 Release Notes: - Fixed the macOS auto-updater leaking a copy of the downloaded update in the system temp directory on every update, and added cleanup of previously leaked files.
This commit is contained in:
1 parent
114307768e
commit
ce21c69d30
1 file changed
+100
-28
@@ -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<Self> {
|
||||
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()
|
||||
|
||||
Reference in new issue
Block a user