From dcdca64d3e9439845a0772eff974476f3dc0d81c Mon Sep 17 00:00:00 2001 From: Random30000 <24901@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:54:49 +0800 Subject: [PATCH] fix(profile): recover Windows lock after process exit --- Cargo.lock | 1 + moli-browser-profile/Cargo.toml | 3 + moli-browser-profile/src/profile_lock.rs | 234 +++++++++++++++++++++-- 3 files changed, 223 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 15c3bb9bcd..17e46742d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2450,6 +2450,7 @@ dependencies = [ "moli-cookie-cache", "serde", "serde_json", + "windows-sys 0.61.2", ] [[package]] diff --git a/moli-browser-profile/Cargo.toml b/moli-browser-profile/Cargo.toml index c49e546a72..5c417bfd0b 100644 --- a/moli-browser-profile/Cargo.toml +++ b/moli-browser-profile/Cargo.toml @@ -11,5 +11,8 @@ moli-cookie-cache = { path = "../moli-cookie-cache" } serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.145" +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61.2", features = ["Win32_Foundation", "Win32_Storage_FileSystem"] } + [lints] workspace = true diff --git a/moli-browser-profile/src/profile_lock.rs b/moli-browser-profile/src/profile_lock.rs index 8bdfab6469..3a8a7408cd 100644 --- a/moli-browser-profile/src/profile_lock.rs +++ b/moli-browser-profile/src/profile_lock.rs @@ -1,24 +1,33 @@ use std::{ fs::{File, OpenOptions}, - io::{Seek, SeekFrom, Write}, + io::{Read, Seek, SeekFrom, Write}, path::{Path, PathBuf}, time::{SystemTime, UNIX_EPOCH}, }; #[cfg(unix)] use std::os::unix::io::AsRawFd; +#[cfg(windows)] +use std::os::windows::io::AsRawHandle; +#[cfg(windows)] +use windows_sys::Win32::{ + Foundation::{ERROR_LOCK_VIOLATION, HANDLE}, + Storage::FileSystem::{LockFile, UnlockFile}, +}; use anyhow::{Context, Result, anyhow}; use crate::BrowserProfilePaths; +const MAX_PROFILE_LOCK_METADATA_BYTES: usize = 1024; + #[derive(Debug)] pub struct BrowserProfileLock { path: PathBuf, - /// Held only on Unix, where the file descriptor keeps the advisory `flock` - /// alive for the lifetime of the guard. On non-Unix platforms the lock is - /// the file's existence on disk, so no handle needs to be retained. - #[cfg(unix)] + /// Held on platforms with process-owned advisory locks. Keeping this + /// handle alive keeps the profile exclusively owned; the OS releases the + /// lock if the process exits without running Rust destructors. + #[cfg(any(unix, windows))] file: File, remove_on_drop: bool, } @@ -35,7 +44,7 @@ impl BrowserProfileLock { impl Drop for BrowserProfileLock { fn drop(&mut self) { - #[cfg(unix)] + #[cfg(any(unix, windows))] let _ = unlock_profile_file(&self.file); if self.remove_on_drop { let _ = std::fs::remove_file(&self.path); @@ -59,7 +68,7 @@ pub fn acquire_profile_lock(paths: &BrowserProfilePaths) -> Result Result<(File, bool } } -#[cfg(not(unix))] +#[cfg(windows)] +fn open_and_lock_profile_file(paths: &BrowserProfilePaths) -> Result<(File, bool)> { + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&paths.lock_path) + .with_context(|| { + format!( + "failed to open browser profile lock `{}`", + paths.lock_path.display() + ) + })?; + match try_lock_profile_file(&file) { + Ok(()) => Ok((file, false)), + Err(error) if is_advisory_lock_contention(&error) => { + let owner = lock_owner_description(&paths.lock_path); + Err(anyhow!( + "browser profile `{}` is already locked by `{}` ({owner})", + paths.root.display(), + paths.lock_path.display() + )) + } + Err(error) => Err(error).with_context(|| { + format!( + "failed to acquire browser profile lock `{}`", + paths.lock_path.display() + ) + }), + } +} + +#[cfg(not(any(unix, windows)))] fn open_and_lock_profile_file(paths: &BrowserProfilePaths) -> Result<(File, bool)> { match OpenOptions::new() .write(true) @@ -175,10 +217,79 @@ fn is_advisory_lock_contention(error: &std::io::Error) -> bool { ) } +#[cfg(windows)] +const WINDOWS_PROFILE_LOCK_OFFSET: u64 = 1_u64 << 32; +#[cfg(windows)] +const WINDOWS_PROFILE_LOCK_LENGTH: u64 = 1; + +#[cfg(windows)] +fn try_lock_profile_file(file: &File) -> std::io::Result<()> { + let (offset_low, offset_high) = split_windows_u64(WINDOWS_PROFILE_LOCK_OFFSET); + let (length_low, length_high) = split_windows_u64(WINDOWS_PROFILE_LOCK_LENGTH); + // SAFETY: the handle is valid for the lifetime of `file`; the locked byte + // lies beyond the bounded metadata prefix so other contenders can still + // read owner diagnostics while the exclusive process lock is held. + let result = unsafe { + LockFile( + file.as_raw_handle() as HANDLE, + offset_low, + offset_high, + length_low, + length_high, + ) + }; + if result != 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(windows)] +fn unlock_profile_file(file: &File) -> std::io::Result<()> { + let (offset_low, offset_high) = split_windows_u64(WINDOWS_PROFILE_LOCK_OFFSET); + let (length_low, length_high) = split_windows_u64(WINDOWS_PROFILE_LOCK_LENGTH); + // SAFETY: this uses the same valid handle and byte range as acquisition. + let result = unsafe { + UnlockFile( + file.as_raw_handle() as HANDLE, + offset_low, + offset_high, + length_low, + length_high, + ) + }; + if result != 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(windows)] +const fn split_windows_u64(value: u64) -> (u32, u32) { + (value as u32, (value >> 32) as u32) +} + +#[cfg(windows)] +fn is_advisory_lock_contention(error: &std::io::Error) -> bool { + error.raw_os_error() == Some(ERROR_LOCK_VIOLATION as i32) +} + fn lock_owner_description(path: &Path) -> String { - let Ok(contents) = std::fs::read_to_string(path) else { + let Ok(file) = File::open(path) else { return "lock owner metadata unavailable".to_owned(); }; + let mut contents = String::new(); + let Ok(bytes_read) = file + .take((MAX_PROFILE_LOCK_METADATA_BYTES + 1) as u64) + .read_to_string(&mut contents) + else { + return "lock owner metadata unavailable".to_owned(); + }; + if bytes_read > MAX_PROFILE_LOCK_METADATA_BYTES { + return "lock owner metadata unavailable".to_owned(); + } let mut fields = Vec::new(); for line in contents.lines() { let line = line.trim(); @@ -203,10 +314,16 @@ mod tests { path::PathBuf, time::{SystemTime, UNIX_EPOCH}, }; + #[cfg(windows)] + use std::{ + process::{Command, Stdio}, + thread, + time::Duration, + }; use anyhow::Result; - use super::BrowserProfileLock; + use super::{BrowserProfileLock, MAX_PROFILE_LOCK_METADATA_BYTES, lock_owner_description}; use crate::BrowserProfilePaths; struct TempProfileDir { @@ -256,22 +373,109 @@ mod tests { ); drop(first); - #[cfg(unix)] + #[cfg(any(unix, windows))] assert!( paths.lock_path.exists(), - "Unix advisory lock metadata file should remain reusable after drop" + "advisory lock metadata file should remain reusable after drop" ); - #[cfg(not(unix))] + #[cfg(not(any(unix, windows)))] assert!( !paths.lock_path.exists(), - "non-Unix sentinel lock file should be removed after drop" + "sentinel lock file should be removed after drop" ); let _second = BrowserProfileLock::acquire(&paths)?; Ok(()) } - #[cfg(unix)] + #[cfg(windows)] + const CHILD_PROFILE_ENV: &str = "MOLI_PROFILE_LOCK_TEST_CHILD_PROFILE"; + #[cfg(windows)] + const CHILD_READY_ENV: &str = "MOLI_PROFILE_LOCK_TEST_CHILD_READY"; + + #[cfg(windows)] + #[test] + fn profile_lock_is_released_when_owner_process_is_terminated() -> Result<()> { + if let (Some(profile), Some(ready)) = ( + std::env::var_os(CHILD_PROFILE_ENV), + std::env::var_os(CHILD_READY_ENV), + ) { + let profile = PathBuf::from(profile); + let ready = PathBuf::from(ready); + let paths = BrowserProfilePaths::new(&profile); + let _lock = BrowserProfileLock::acquire(&paths)?; + fs::write(&ready, b"ready")?; + thread::sleep(Duration::from_secs(120)); + return Ok(()); + } + + let profile = TempProfileDir::new("terminated-owner"); + let ready = profile.path.join("child-ready"); + let test_name = + "profile_lock::tests::profile_lock_is_released_when_owner_process_is_terminated"; + let mut child = Command::new(std::env::current_exe()?) + .args(["--exact", test_name, "--nocapture"]) + .env(CHILD_PROFILE_ENV, &profile.path) + .env(CHILD_READY_ENV, &ready) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn()?; + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while !ready.exists() { + if let Some(status) = child.try_wait()? { + anyhow::bail!("profile lock child exited before readiness: {status}"); + } + if std::time::Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + anyhow::bail!("timed out waiting for profile lock child readiness"); + } + thread::sleep(Duration::from_millis(25)); + } + + let paths = BrowserProfilePaths::new(&profile.path); + let error = BrowserProfileLock::acquire(&paths) + .expect_err("live child must keep the profile exclusively locked"); + assert!(error.to_string().contains("already locked")); + let child_pid = child.id(); + + child.kill()?; + let _ = child.wait()?; + + let reopened = BrowserProfileLock::acquire(&paths)?; + let metadata = fs::read_to_string(&paths.lock_path)?; + assert!(metadata.contains(&format!("pid={}", std::process::id()))); + assert!( + !metadata + .lines() + .any(|line| line == format!("pid={child_pid}")), + "terminated owner metadata should be replaced: {metadata}" + ); + drop(reopened); + assert!(paths.lock_path.exists()); + Ok(()) + } + + #[test] + fn profile_lock_owner_metadata_read_is_bounded() -> Result<()> { + let profile = TempProfileDir::new("bounded-owner-metadata"); + let paths = BrowserProfilePaths::new(&profile.path); + fs::create_dir_all(&profile.path)?; + fs::write( + &paths.lock_path, + "pid=1\ncreated_unix_ms=1\n".repeat(MAX_PROFILE_LOCK_METADATA_BYTES), + )?; + + assert_eq!( + lock_owner_description(&paths.lock_path), + "lock owner metadata unavailable" + ); + Ok(()) + } + + #[cfg(any(unix, windows))] #[test] fn profile_lock_reuses_existing_unlocked_lock_file() -> Result<()> { let profile = TempProfileDir::new("stale-file");