From 334b6a574a598e99b63754902a7d04f8cfecc6a1 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Mon, 21 Sep 2026 12:04:10 +0800 Subject: [PATCH] fix(plugins): retry plugin store rename on transient Windows locks --- .../src/plugins/installer.rs | 104 +++++++++++++++++- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/crates/dbx-plugin-runtime/src/plugins/installer.rs b/crates/dbx-plugin-runtime/src/plugins/installer.rs index 2ae8dd9ab4..db41b2b887 100644 --- a/crates/dbx-plugin-runtime/src/plugins/installer.rs +++ b/crates/dbx-plugin-runtime/src/plugins/installer.rs @@ -3,6 +3,7 @@ use std::fs::{File, OpenOptions}; use std::io::{Cursor, Read, Write}; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; +use std::time::Duration; use base64::Engine; use chrono::Utc; @@ -554,10 +555,10 @@ impl PluginPackageInstaller { } else { None }; - if let Err(error) = std::fs::rename(&package_dir, &version_dir) { + if let Err(error) = rename_with_transient_lock_retry(&package_dir, &version_dir) { let mut message = format!("Failed to store plugin '{}' version {}: {error}", manifest.id, version); if let Some(backup) = &replaced_version_dir { - if let Err(restore) = std::fs::rename(backup, &version_dir) { + if let Err(restore) = rename_with_transient_lock_retry(backup, &version_dir) { message.push_str(&format!("; previous copy kept at {}: {restore}", backup.display())); } } @@ -600,7 +601,7 @@ impl PluginPackageInstaller { let _ = std::fs::remove_dir_all(&version_dir); let mut message = error; if let Some(backup) = &replaced_version_dir { - if let Err(restore) = std::fs::rename(backup, &version_dir) { + if let Err(restore) = rename_with_transient_lock_retry(backup, &version_dir) { message.push_str(&format!("; previous copy kept at {}: {restore}", backup.display())); } } @@ -1059,6 +1060,34 @@ fn open_install_lock(root_dir: &Path) -> Result { .map_err(|error| error.to_string()) } +// Antivirus scanners briefly hold handles on freshly extracted plugin binaries, which fails +// directory renames with ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32); the scan +// normally finishes well inside this retry window. +const TRANSIENT_LOCK_RETRY_DELAYS: [Duration; 3] = + [Duration::from_millis(200), Duration::from_millis(400), Duration::from_millis(800)]; + +fn rename_with_transient_lock_retry(src: &Path, dst: &Path) -> std::io::Result<()> { + retry_transient_lock(&TRANSIENT_LOCK_RETRY_DELAYS, || std::fs::rename(src, dst)) +} + +fn retry_transient_lock( + delays: &[Duration], + mut operation: impl FnMut() -> std::io::Result, +) -> std::io::Result { + for delay in delays { + match operation() { + Ok(value) => return Ok(value), + Err(error) if cfg!(windows) && is_windows_lock_error(&error) => std::thread::sleep(*delay), + Err(error) => return Err(error), + } + } + operation() +} + +fn is_windows_lock_error(error: &std::io::Error) -> bool { + matches!(error.raw_os_error(), Some(5 | 32)) +} + fn make_backend_executable(path: &Option) -> Result<(), String> { #[cfg(unix)] if let Some(path) = path { @@ -1170,15 +1199,17 @@ mod tests { use std::collections::{BTreeMap, BTreeSet}; use std::io::{Cursor, Write}; use std::path::Path; + use std::time::Duration; use base64::Engine; use ed25519_dalek::{Signer, SigningKey}; use zip::write::SimpleFileOptions; use super::{ - is_activation_record_file, read_install_identity, sha256_hex, validate_package_expectation, - PluginInstallPolicy, PluginPackageExpectation, PluginPackageInstaller, PluginSignatureStatus, PluginTrustStore, - ACTIVATIONS_DIR, PLUGIN_CHECKSUMS_FILE, PLUGIN_SIGNATURE_FILE, VERSIONS_DIR, + is_activation_record_file, read_install_identity, retry_transient_lock, sha256_hex, + validate_package_expectation, PluginInstallPolicy, PluginPackageExpectation, PluginPackageInstaller, + PluginSignatureStatus, PluginTrustStore, ACTIVATIONS_DIR, PLUGIN_CHECKSUMS_FILE, PLUGIN_SIGNATURE_FILE, + VERSIONS_DIR, }; use crate::plugins::{PluginManifest, PluginRegistry}; @@ -2001,4 +2032,65 @@ mod tests { } output.into_inner() } + + #[test] + #[cfg(windows)] + fn transient_lock_retry_recovers_from_windows_lock_errors() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + for code in [5, 32] { + let attempts = AtomicUsize::new(0); + let result = retry_transient_lock(&[Duration::ZERO, Duration::ZERO, Duration::ZERO], || { + if attempts.fetch_add(1, Ordering::SeqCst) < 2 { + Err(std::io::Error::from_raw_os_error(code)) + } else { + Ok(()) + } + }); + assert!(result.is_ok()); + assert_eq!(attempts.load(Ordering::SeqCst), 3); + } + } + + #[test] + #[cfg(windows)] + fn transient_lock_retry_returns_last_error_after_exhausting_delays() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let attempts = AtomicUsize::new(0); + let result: std::io::Result<()> = retry_transient_lock(&[Duration::ZERO], || { + attempts.fetch_add(1, Ordering::SeqCst); + Err(std::io::Error::from_raw_os_error(5)) + }); + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(result.unwrap_err().raw_os_error(), Some(5)); + } + + #[test] + fn transient_lock_retry_does_not_retry_other_errors() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let attempts = AtomicUsize::new(0); + let result: std::io::Result<()> = retry_transient_lock(&[Duration::ZERO; 3], || { + attempts.fetch_add(1, Ordering::SeqCst); + Err(std::io::Error::from_raw_os_error(2)) + }); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + assert_eq!(result.unwrap_err().raw_os_error(), Some(2)); + } + + #[test] + #[cfg(unix)] + fn transient_lock_retry_treats_windows_lock_codes_as_final_on_unix() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + // Raw code 5 on Unix is EIO, not a Windows lock error, so the retry gate must leave it final. + let attempts = AtomicUsize::new(0); + let result: std::io::Result<()> = retry_transient_lock(&[Duration::ZERO; 3], || { + attempts.fetch_add(1, Ordering::SeqCst); + Err(std::io::Error::from_raw_os_error(5)) + }); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + assert_eq!(result.unwrap_err().raw_os_error(), Some(5)); + } }