mirror of
https://github.com/lexmount/moli.git
synced 2026-10-03 08:00:49 +00:00
fix(profile): recover Windows lock after process exit
This commit is contained in:
Generated
+1
@@ -2450,6 +2450,7 @@ dependencies = [
|
||||
"moli-cookie-cache",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<BrowserProfil
|
||||
|
||||
Ok(BrowserProfileLock {
|
||||
path: paths.lock_path.clone(),
|
||||
#[cfg(unix)]
|
||||
#[cfg(any(unix, windows))]
|
||||
file,
|
||||
remove_on_drop,
|
||||
})
|
||||
@@ -98,7 +107,40 @@ fn open_and_lock_profile_file(paths: &BrowserProfilePaths) -> 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");
|
||||
|
||||
Reference in New Issue
Block a user