Unmount the auto-update disk image regardless of the auto-update status (#17019)
Closes #10782 In some cases, during the auto-update process, the update can fail and leave a dangling disk image in macOS. If the auto-update fails again, a new dangling mounted volume will be left behind. To avoid polluting the system with these dangling mounted disk images, implement [the `Drop` trait](https://doc.rust-lang.org/std/ops/trait.Drop.html) for the `MacOSUnmounter` struct. This will ensure that the disk image is unmounted when the `install_release_macos` function exits regardless of its result. ## How to test this locally Unfortunately, I was a bit too lazy to find a smarter way to test this, so I simply commented out a bunch of lines to emulate the auto-update process. To replicate the linked issue (#10782), you can apply the attached patch. Build the Zed binary and run it. The auto-update should fail, leaving the dangling mounted disk image in the system: ```shell >diskutil list /dev/disk5 (synthesized): #: TYPE NAME SIZE IDENTIFIER 0: APFS Container Scheme - +220.6 MB disk5 Physical Store disk4s1 1: APFS Volume Zed 190.6 MB disk5s1 ``` Run the Zed binary again to create another mounted disk image: ```shell >diskutil list /dev/disk5 (synthesized): #: TYPE NAME SIZE IDENTIFIER 0: APFS Container Scheme - +220.6 MB disk5 Physical Store disk4s1 1: APFS Volume Zed 190.6 MB disk5s1 /dev/disk7 (synthesized): #: TYPE NAME SIZE IDENTIFIER 0: APFS Container Scheme - +220.6 MB disk7 Physical Store disk6s1 1: APFS Volume Zed 190.6 MB disk7s1 ``` [simulate_zed_autoupdate.patch](https://github.com/user-attachments/files/16787955/simulate_zed_autoupdate.patch) Please let me know if the fix is good; otherwise, I am happy to implement it differently. Thanks! Release Notes: - Fixed #10782
This commit is contained in:
parent
760e1a6db0
commit
ca2eb275de
1 changed files with 33 additions and 12 deletions
|
@ -88,6 +88,34 @@ struct JsonRelease {
|
||||||
url: String,
|
url: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct MacOsUnmounter {
|
||||||
|
mount_path: PathBuf,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Drop for MacOsUnmounter {
|
||||||
|
fn drop(&mut self) {
|
||||||
|
let unmount_output = std::process::Command::new("hdiutil")
|
||||||
|
.args(&["detach", "-force"])
|
||||||
|
.arg(&self.mount_path)
|
||||||
|
.output();
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
struct AutoUpdateSetting(bool);
|
struct AutoUpdateSetting(bool);
|
||||||
|
|
||||||
/// Whether or not to automatically check for updates.
|
/// Whether or not to automatically check for updates.
|
||||||
|
@ -739,6 +767,11 @@ async fn install_release_macos(
|
||||||
String::from_utf8_lossy(&output.stderr)
|
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 {
|
||||||
|
mount_path: mount_path.clone(),
|
||||||
|
};
|
||||||
|
|
||||||
let output = Command::new("rsync")
|
let output = Command::new("rsync")
|
||||||
.args(&["-av", "--delete"])
|
.args(&["-av", "--delete"])
|
||||||
.arg(&mounted_app_path)
|
.arg(&mounted_app_path)
|
||||||
|
@ -752,17 +785,5 @@ async fn install_release_macos(
|
||||||
String::from_utf8_lossy(&output.stderr)
|
String::from_utf8_lossy(&output.stderr)
|
||||||
);
|
);
|
||||||
|
|
||||||
let output = Command::new("hdiutil")
|
|
||||||
.args(&["detach"])
|
|
||||||
.arg(&mount_path)
|
|
||||||
.output()
|
|
||||||
.await?;
|
|
||||||
|
|
||||||
anyhow::ensure!(
|
|
||||||
output.status.success(),
|
|
||||||
"failed to unount: {:?}",
|
|
||||||
String::from_utf8_lossy(&output.stderr)
|
|
||||||
);
|
|
||||||
|
|
||||||
Ok(running_app_path)
|
Ok(running_app_path)
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue