From ca2eb275de80133c6a3e29a77609e82798d7c38d Mon Sep 17 00:00:00 2001 From: Vitaly Slobodin Date: Thu, 29 Aug 2024 06:15:38 +0200 Subject: [PATCH] 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 --- crates/auto_update/src/auto_update.rs | 45 ++++++++++++++++++++------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 6e52154c1b..4474357dd6 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -88,6 +88,34 @@ struct JsonRelease { 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); /// Whether or not to automatically check for updates. @@ -739,6 +767,11 @@ 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 { + mount_path: mount_path.clone(), + }; + let output = Command::new("rsync") .args(&["-av", "--delete"]) .arg(&mounted_app_path) @@ -752,17 +785,5 @@ async fn install_release_macos( 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) }