From fc93bd091e7810e08b540a67487e3bf5f2b7da2b Mon Sep 17 00:00:00 2001 From: Agapenor <120006055+Agapenordev@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:46:31 +0000 Subject: [PATCH] Wait for browser shutdown before cleaning private profiles (#827) Thanks @Agapenordev! Supervised browser hand-off now waits for process-group/Job Object shutdown, and private-profile removal retries transient locks for up to five seconds. Missing profiles count as already cleaned. Reviewed source head 6e3398d28be068bcc96c16d197b8134dfce813ee. Hosted Ubuntu CI is green. On AnayPC, the applicable engine/platform tests, clippy, layers, locks, secrets and wasm checks pass. Two Windows UI-automation tests also fail on pristine main because the SSH session cannot create interactive WinForms windows; their baseline and candidate failures are preserved. Related #810 remains open: this follow-up does not establish that the broader live macOS/Brave issue is fully resolved. --- crates/engine/src/tools/web_common.rs | 31 ++++++++++++++++++++++++--- crates/platform/src/process.rs | 2 +- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/tools/web_common.rs b/crates/engine/src/tools/web_common.rs index 7606fb2d..8c0502f9 100644 --- a/crates/engine/src/tools/web_common.rs +++ b/crates/engine/src/tools/web_common.rs @@ -58,11 +58,21 @@ pub struct Browser { struct BrowserProfile(std::path::PathBuf); +fn remove_profile_with(mut remove: impl FnMut() -> std::io::Result<()>) -> std::io::Result<()> { + let started = std::time::Instant::now(); + loop { + match remove() { + Ok(()) => return Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(e) if started.elapsed() >= Duration::from_secs(5) => return Err(e), + Err(_) => std::thread::sleep(Duration::from_millis(50)), + } + } +} + impl Drop for BrowserProfile { fn drop(&mut self) { - if let Err(e) = std::fs::remove_dir_all(&self.0) - && e.kind() != std::io::ErrorKind::NotFound - { + if let Err(e) = remove_profile_with(|| std::fs::remove_dir_all(&self.0)) { log::warn!("couldn't remove the private browser profile {}: {e}", self.0.display()); } } @@ -1081,6 +1091,21 @@ mod tests { assert!(!profile.exists(), "the private browser profile survived its owner"); } + #[test] + fn profile_cleanup_retries_while_the_browser_releases_files() { + let root = tempfile::tempdir().unwrap(); + let profile = root.path().join("private-profile"); + std::fs::create_dir(&profile).unwrap(); + let mut attempts = 0; + super::remove_profile_with(|| { + attempts += 1; + if attempts < 3 { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "still locked")) } else { std::fs::remove_dir_all(&profile) } + }) + .unwrap(); + assert_eq!(attempts, 3); + assert!(!profile.exists()); + } + /// #747: a Flatpak or Snap browser keeps its profile where its sandbox can reach it and we can /// read its port; any other browser uses the temp folder. #[test] diff --git a/crates/platform/src/process.rs b/crates/platform/src/process.rs index 70501180..c297e05b 100644 --- a/crates/platform/src/process.rs +++ b/crates/platform/src/process.rs @@ -758,7 +758,7 @@ impl Supervisor { let _ = self.child.kill(); } else { // The leader exited; take any leftover group members with it so they can't hold the pipes. - let _ = self.child.start_kill(); + let _ = self.child.kill(); } self.drain(); let duration = self.start.elapsed();