diff --git a/crates/dbx-core/src/agent_manager.rs b/crates/dbx-core/src/agent_manager.rs index ed7edc3fc2..4cdbe9cbfa 100644 --- a/crates/dbx-core/src/agent_manager.rs +++ b/crates/dbx-core/src/agent_manager.rs @@ -62,6 +62,63 @@ mod tests { } } + #[test] + fn cleanup_pending_jre_removes_stash_dirs_and_persists() { + let manager = test_manager("pending-cleanup"); + std::fs::create_dir_all(manager.base_dir()).unwrap(); + let stash = manager.base_dir().join("jre-21.old-1700000000-deadbeef"); + std::fs::create_dir_all(&stash).unwrap(); + std::fs::write(stash.join("dummy"), b"x").unwrap(); + + let mut state = AgentState::default(); + state.pending_jre_cleanup.push(stash.clone()); + manager.save_state(&state).unwrap(); + + // Re-create the manager (simulates app restart). + let manager2 = AgentManager::new_with_base_dir(manager.base_dir().clone()); + + assert!(!stash.exists(), "stash dir should be removed"); + let after = manager2.load_state(); + assert!(after.pending_jre_cleanup.is_empty(), "cleanup should drain the list on success"); + } + + #[test] + fn cleanup_orphan_jre_dirs_removes_unrecorded_stash() { + let manager = test_manager("orphan-cleanup"); + std::fs::create_dir_all(manager.base_dir()).unwrap(); + let orphan = manager.base_dir().join("jre-21.old-1234567890-cafe"); + std::fs::create_dir_all(&orphan).unwrap(); + + // Re-create manager — it should sweep orphans even without state. + let _manager2 = AgentManager::new_with_base_dir(manager.base_dir().clone()); + + assert!(!orphan.exists(), "orphan stash should be swept"); + } + + #[test] + fn cleanup_skips_active_jre_dir() { + let manager = test_manager("orphan-skip-active"); + std::fs::create_dir_all(manager.base_dir()).unwrap(); + let active = manager.jre_dir(DEFAULT_JRE_KEY); // jre-21 + std::fs::create_dir_all(&active).unwrap(); + + let _manager2 = AgentManager::new_with_base_dir(manager.base_dir().clone()); + + assert!(active.exists(), "active jre- dir must not be touched (no .old- in name)"); + } + + #[test] + fn agent_state_back_compat_without_pending_jre_cleanup() { + // Old state JSON without pending_jre_cleanup must still deserialize. + let json = r#"{ + "jre_versions": {}, + "installed_drivers": {}, + "java_runtime": {"mode": "managed"} + }"#; + let state: AgentState = serde_json::from_str(json).expect("deserialize legacy state"); + assert!(state.pending_jre_cleanup.is_empty()); + } + #[test] fn resolves_managed_java_runtime_by_default() { let manager = test_manager("managed"); @@ -241,6 +298,11 @@ pub struct AgentState { pub installed_drivers: std::collections::HashMap, #[serde(default)] pub java_runtime: JavaRuntimeConfig, + /// Old JRE directories that could not be deleted in-place during a + /// reinstall on Windows; renamed aside (`.old--`) and + /// cleaned up best-effort on next `AgentManager::new`. + #[serde(default)] + pub pending_jre_cleanup: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -336,6 +398,8 @@ impl AgentManager { let mgr = Self { base_dir, app_version: app_version.into(), daemons: Mutex::new(std::collections::HashMap::new()) }; mgr.migrate_legacy_jre(); + mgr.cleanup_pending_jre_dirs(); + mgr.cleanup_orphan_jre_dirs(); mgr } @@ -347,6 +411,62 @@ impl AgentManager { } } + /// Best-effort cleanup of `pending_jre_cleanup` paths recorded by previous + /// runs that fell back to renaming an old JRE aside on Windows. Successful + /// removals are pruned from the persisted state. Failures are kept for the + /// next launch and never block startup. (Issue #1100, D6.) + fn cleanup_pending_jre_dirs(&self) { + let mut state = self.load_state(); + if state.pending_jre_cleanup.is_empty() { + return; + } + let mut remaining = Vec::new(); + for path in std::mem::take(&mut state.pending_jre_cleanup) { + if !path.exists() { + continue; + } + match std::fs::remove_dir_all(&path) { + Ok(()) => log::info!("Cleaned up pending JRE stash: {}", path.display()), + Err(err) => { + log::warn!("Pending JRE cleanup failed for {}: {err}", path.display()); + remaining.push(path); + } + } + } + state.pending_jre_cleanup = remaining; + if let Err(err) = self.save_state(&state) { + log::warn!("Failed to persist post-cleanup AgentState: {err}"); + } + } + + /// Sweep `base_dir` for orphan `*.old-*` JRE stash directories left behind + /// by previous runs (e.g. process crashed before the stash was recorded). + /// Best-effort — failures are ignored. + fn cleanup_orphan_jre_dirs(&self) { + let entries = match std::fs::read_dir(&self.base_dir) { + Ok(e) => e, + Err(_) => return, + }; + for entry in entries.flatten() { + let Some(name) = entry.file_name().to_str().map(str::to_string) else { + continue; + }; + // Match `<...>.old--<...>` (typically `jre-21.old-...`), + // which is the suffix scheme used by stash_old_jre_dir. + if !name.starts_with("jre-") || !name.contains(".old-") { + continue; + } + let path = entry.path(); + if !path.is_dir() { + continue; + } + match std::fs::remove_dir_all(&path) { + Ok(()) => log::info!("Cleaned up orphan JRE stash: {}", path.display()), + Err(err) => log::warn!("Orphan JRE cleanup failed for {}: {err}", path.display()), + } + } + } + pub fn base_dir(&self) -> &PathBuf { &self.base_dir } diff --git a/crates/dbx-core/src/agent_service.rs b/crates/dbx-core/src/agent_service.rs index dd873b4b99..6ed0d7b9b6 100644 --- a/crates/dbx-core/src/agent_service.rs +++ b/crates/dbx-core/src/agent_service.rs @@ -1,12 +1,133 @@ use std::hash::{Hash, Hasher}; use std::io::Read; use std::path::{Path, PathBuf}; +use std::time::Duration; use crate::agent_catalog; use crate::agent_manager::{ AgentDriverInfo, AgentManager, AgentRegistry, InstalledDriver, JavaRuntimeMode, DEFAULT_JRE_KEY, }; +/// Number of attempts to delete a JRE directory before giving up (Windows +/// experiences transient `ERROR_ACCESS_DENIED` when java.exe is still mapped +/// or anti-virus is scanning the archive). POSIX returns 1 — `unlink` of an +/// in-use file always succeeds. +const JRE_REMOVE_ATTEMPTS: usize = if cfg!(windows) { 6 } else { 1 }; + +/// Exponential-ish backoff between retries. Total wait ≈ 1.55s on Windows. +const JRE_REMOVE_BACKOFF_MS: &[u64] = &[50, 100, 200, 400, 400, 400]; + +/// Delete an old JRE directory, retrying on Windows to cover the daemon-exit +/// and AV-scan release window. Returns the original `std::io::Error` when all +/// retries fail so callers can decide whether to fall back to rename-stash. +fn remove_jre_dir_with_retry(path: &Path) -> std::io::Result<()> { + if !path.exists() { + return Ok(()); + } + let mut last_err: Option = None; + for i in 0..JRE_REMOVE_ATTEMPTS { + match std::fs::remove_dir_all(path) { + Ok(()) => return Ok(()), + Err(err) => { + log::warn!( + "remove_dir_all({}) attempt {}/{} failed: {err}", + path.display(), + i + 1, + JRE_REMOVE_ATTEMPTS + ); + last_err = Some(err); + if i + 1 < JRE_REMOVE_ATTEMPTS { + let delay_ms = JRE_REMOVE_BACKOFF_MS.get(i).copied().unwrap_or(400); + std::thread::sleep(Duration::from_millis(delay_ms)); + } + } + } + } + Err(last_err.unwrap_or_else(|| std::io::Error::other("remove_dir_all failed without an error"))) +} + +/// Render a friendly Chinese error message when the old JRE directory cannot +/// be replaced. On Windows, lists likely culprits (process holding java.exe, +/// AV scanning) and suggests restarting dbx; on POSIX returns a concise +/// message. The original OS error is appended in parentheses for support. +fn format_jre_dir_remove_error(path: &Path, os_err: &std::io::Error) -> String { + if cfg!(windows) { + format!( + "无法删除旧的 JRE 目录：{}\n\ + 可能的原因：\n \ + - 仍有 dbx Agent / java 进程占用该目录\n \ + - 防病毒软件正在扫描\n\ + 请关闭可能持有该目录的进程，或重启 dbx 后重试。\n\ + （原始错误：{os_err}）", + path.display() + ) + } else { + format!("无法删除旧的 JRE 目录：{}（原始错误：{os_err}）", path.display()) + } +} + +/// Windows-only: rename the old JRE dir to a unique sibling so the install +/// can continue even when files inside are still mapped. Returns the stash +/// path so the caller can record it for later cleanup. On POSIX this is +/// unreachable (callers gate on `cfg(windows)` after a failed remove). +#[cfg(windows)] +fn stash_old_jre_dir(path: &Path) -> std::io::Result { + use std::time::SystemTime; + + let file_name = path + .file_name() + .and_then(|name| name.to_str()) + .ok_or_else(|| std::io::Error::other("JRE directory has no file name"))?; + let ts = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0); + // uuid::Uuid::new_v4() is already a workspace dependency — use its short + // form for a unique suffix without pulling in `rand`. + let rand = uuid::Uuid::new_v4().simple().to_string(); + let stash = path.with_file_name(format!("{file_name}.old-{ts}-{rand}")); + std::fs::rename(path, &stash)?; + Ok(stash) +} + +/// Replace an old JRE directory in-place: try retried `remove_dir_all` first; +/// on Windows fall back to rename-stash if removal fails. Returns the stash +/// path (Some) if the rename fallback was used so the caller can persist it +/// for startup cleanup, or None if the directory was deleted outright (or +/// did not exist). +fn replace_old_jre_dir(am: &AgentManager, path: &Path) -> Result, String> { + match remove_jre_dir_with_retry(path) { + Ok(()) => Ok(None), + Err(remove_err) => { + #[cfg(windows)] + { + match stash_old_jre_dir(path) { + Ok(stash) => { + log::warn!("remove_dir_all failed, stashed old JRE at {} ({remove_err})", stash.display()); + // Persist immediately so a crash before extraction + // still leaves the stash recorded for cleanup. + let mut state = am.load_state(); + state.pending_jre_cleanup.push(stash.clone()); + if let Err(save_err) = am.save_state(&state) { + log::warn!("Failed to persist pending_jre_cleanup: {save_err}"); + } + Ok(Some(stash)) + } + Err(rename_err) => { + log::warn!( + "remove_dir_all and rename both failed for {}: remove={remove_err}, rename={rename_err}", + path.display() + ); + Err(format_jre_dir_remove_error(path, &remove_err)) + } + } + } + #[cfg(not(windows))] + { + let _ = am; // silence unused warning on POSIX + Err(format_jre_dir_remove_error(path, &remove_err)) + } + } + } +} + const REGISTRY_PATH: &str = "https://github.com/t8y2/dbx-agents/releases/latest/download/agent-registry.json"; const REGISTRY_R2_PATH: &str = "agents/agent-registry.json"; @@ -261,14 +382,16 @@ pub async fn uninstall_agent_jre(am: &AgentManager, jre_key: &str) -> Result<(), if !dependents.is_empty() { return Err(format!("JRE {} 正在被以下驱动使用: {}，请先卸载这些驱动", jre_key, dependents.join(", "))); } + // Stop daemons first so any java.exe holding the JRE files exits before + // we try to remove the directory (Windows ERROR_ACCESS_DENIED otherwise). + am.stop_daemons().await; let jre_dir = am.jre_dir(jre_key); - if jre_dir.exists() { - std::fs::remove_dir_all(&jre_dir).map_err(|err| format!("Failed to remove JRE: {err}"))?; + if let Err(err) = remove_jre_dir_with_retry(&jre_dir) { + return Err(format_jre_dir_remove_error(&jre_dir, &err)); } let mut local_state = am.load_state(); local_state.jre_versions.remove(jre_key); am.save_state(&local_state)?; - am.stop_daemons().await; Ok(()) } @@ -300,15 +423,16 @@ pub async fn reinstall_agent_jre( ) .await?; let jre_dir = am.jre_dir(jre_key); - if jre_dir.exists() { - std::fs::remove_dir_all(&jre_dir).map_err(|err| format!("Failed to remove old JRE: {err}"))?; - } + // Stop daemons before deleting so java.exe processes release file + // handles on Windows (Issue #1100). Falls back to a rename-stash if the + // directory still cannot be removed. + am.stop_daemons().await; + replace_old_jre_dir(am, &jre_dir)?; extract_tar_gz(&jre_archive, &jre_dir)?; std::fs::remove_file(&jre_archive).ok(); let mut local_state = am.load_state(); local_state.jre_versions.insert(jre_key.to_string(), jre_info.version.clone()); am.save_state(&local_state)?; - am.stop_daemons().await; progress(AgentProgressEvent::step("done")); Ok(()) } @@ -406,9 +530,9 @@ async fn install_agent_driver_from_registry( .await?; progress(AgentProgressEvent::transfer("jre-extract", 0, 0).with_batch(Some(db_type), current, total_drivers)); let jre_dir = am.jre_dir(jre_key); - if jre_dir.exists() { - std::fs::remove_dir_all(&jre_dir).map_err(|err| format!("Failed to remove old JRE: {err}"))?; - } + // Stop daemons first (Windows ERROR_ACCESS_DENIED, Issue #1100). + am.stop_daemons().await; + replace_old_jre_dir(am, &jre_dir)?; extract_tar_gz(&jre_archive, &jre_dir)?; std::fs::remove_file(&jre_archive).ok(); } @@ -792,9 +916,11 @@ pub fn import_offline_zip( } let jre_dir = am.jre_dir(jre_key); - if jre_dir.exists() { - std::fs::remove_dir_all(&jre_dir).ok(); - } + // Daemons cannot be stopped from a sync function safely; the retry + + // Windows rename fallback in replace_old_jre_dir still handles a + // locked directory. Daemon shutdown for the foreground install paths + // happens in `reinstall_agent_jre` and `install_agent_driver_*`. + replace_old_jre_dir(am, &jre_dir)?; extract_tar_gz(&tmp_archive, &jre_dir)?; std::fs::remove_file(&tmp_archive).ok(); @@ -911,3 +1037,70 @@ pub fn import_agent_jar(am: &AgentManager, db_type: &str, jar_path: &Path) -> Re } install_local_agent(am, db_type, jar_path.to_path_buf()) } + +// ──────────── Tests ──────────── + +#[cfg(test)] +mod jre_dir_remove_tests { + use super::*; + + fn unique_tmp(name: &str) -> PathBuf { + std::env::temp_dir().join(format!("dbx-jre-remove-{name}-{}", uuid::Uuid::new_v4())) + } + + #[test] + fn remove_returns_ok_when_path_missing() { + let path = unique_tmp("missing"); + assert!(!path.exists()); + assert!(remove_jre_dir_with_retry(&path).is_ok()); + } + + #[test] + fn remove_deletes_existing_dir() { + let dir = unique_tmp("happy"); + std::fs::create_dir_all(dir.join("bin")).unwrap(); + std::fs::write(dir.join("bin").join("java"), b"x").unwrap(); + assert!(dir.exists()); + remove_jre_dir_with_retry(&dir).expect("happy path delete"); + assert!(!dir.exists()); + } + + #[test] + fn windows_error_message_lists_root_causes_and_path() { + let path = PathBuf::from("/tmp/dbx-jre-test"); + let err = std::io::Error::new(std::io::ErrorKind::PermissionDenied, "拒绝访问。 (os error 5)"); + let rendered = format_jre_dir_remove_error(&path, &err); + assert!(rendered.contains(&path.display().to_string()), "missing path: {rendered}"); + assert!(rendered.contains("（原始错误："), "missing original error wrapper: {rendered}"); + assert!(rendered.contains("拒绝访问"), "missing original error text: {rendered}"); + if cfg!(windows) { + assert!(rendered.starts_with("无法删除旧的 JRE 目录："), "wrong prefix: {rendered}"); + assert!(rendered.contains("Agent / java 进程占用"), "missing process advice: {rendered}"); + assert!(rendered.contains("重启 dbx 后重试"), "missing restart advice: {rendered}"); + } else { + // POSIX path: short form, no Windows-specific advice. + assert!(rendered.contains("无法删除旧的 JRE 目录")); + assert!(!rendered.contains("防病毒")); + } + } + + #[test] + #[cfg(windows)] + fn stash_old_jre_dir_renames_and_is_unique() { + let base = unique_tmp("stash-unique"); + std::fs::create_dir_all(&base).unwrap(); + let jre_a = base.join("jre-21"); + std::fs::create_dir_all(&jre_a).unwrap(); + let stash_a = stash_old_jre_dir(&jre_a).expect("first stash"); + assert!(stash_a.exists(), "stash dir should exist after rename"); + assert!(!jre_a.exists(), "original dir should be gone after rename"); + + // Recreate original and stash again — name must differ. + std::fs::create_dir_all(&jre_a).unwrap(); + let stash_b = stash_old_jre_dir(&jre_a).expect("second stash"); + assert_ne!(stash_a, stash_b, "stash names must be unique across calls"); + + // Cleanup. + let _ = std::fs::remove_dir_all(&base); + } +}