fix(profile): simplify file writes for Windows

This commit is contained in:
ldm0
2026-09-19 21:56:20 +08:00
committed by Donough Liu
parent 8668a6f9fc
commit d4f5e2fc01
8 changed files with 61 additions and 51 deletions
+19
View File
@@ -143,6 +143,25 @@ jobs:
- name: Run full workspace tests
run: cargo nextest run --workspace --profile ci
windows-browser-profile:
name: Windows browser profile
runs-on: windows-2025
timeout-minutes: 20
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Install pinned Rust toolchain
shell: pwsh
run: |
$ErrorActionPreference = "Stop"
$PSNativeCommandUseErrorActionPreference = $true
$toolchain = (Get-Content rust-toolchain -Raw).Trim()
rustup toolchain install $toolchain --profile minimal --no-self-update
- name: Test browser profile persistence on Windows
run: cargo test --locked -p moli-browser-profile
python-harness-tests:
name: Python harness tests
runs-on: ubuntu-latest
+2 -2
View File
@@ -1,6 +1,6 @@
mod atomic_write;
mod identity;
mod profile;
mod profile_file;
mod profile_lock;
mod profile_manifest;
mod profile_partition_id;
@@ -8,13 +8,13 @@ mod profile_paths;
mod user_agent_override;
mod window_surface;
pub use atomic_write::write_file_atomically;
pub use identity::{
BrowserBrandVersion, BrowserIdentityProfile, BrowserUserAgentMetadataOverride,
parse_accept_language,
};
pub use moli_cookie_cache::{load_cookie_cache, save_cookie_cache};
pub use profile::{BrowserProfile, BrowserProfilePartition};
pub use profile_file::write_profile_file;
pub use profile_lock::{BrowserProfileLock, acquire_profile_lock};
pub use profile_manifest::{
BrowserProfileManifest, BrowserProfilePartitionManifest, PROFILE_MANIFEST_VERSION,
@@ -1,7 +1,6 @@
use std::{
ffi::OsString,
fs::{self, OpenOptions},
io::Write,
fs,
path::{Path, PathBuf},
sync::atomic::{AtomicU64, Ordering},
time::{SystemTime, UNIX_EPOCH},
@@ -11,9 +10,9 @@ use anyhow::{Context, Result};
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
/// Atomically replaces a profile file and makes both its bytes and directory
/// entry durable before reporting success.
pub fn write_file_atomically(path: &Path, bytes: &[u8], label: &str) -> Result<()> {
/// Writes a profile file via a temporary file without forcing a disk sync.
/// The completed write replaces the target; crash durability is not guaranteed.
pub fn write_profile_file(path: &Path, bytes: &[u8], label: &str) -> Result<()> {
if let Some(parent) = path.parent()
&& !parent.as_os_str().is_empty()
{
@@ -23,24 +22,15 @@ pub fn write_file_atomically(path: &Path, bytes: &[u8], label: &str) -> Result<(
let tmp_path = unique_temp_path(path);
let result = (|| {
let mut tmp = OpenOptions::new()
.create_new(true)
.write(true)
.open(&tmp_path)
.with_context(|| format!("failed to create {label} `{}`", tmp_path.display()))?;
tmp.write_all(bytes)
fs::write(&tmp_path, bytes)
.with_context(|| format!("failed to write {label} `{}`", tmp_path.display()))?;
tmp.sync_all()
.with_context(|| format!("failed to sync {label} `{}`", tmp_path.display()))?;
drop(tmp);
fs::rename(&tmp_path, path).with_context(|| {
format!(
"failed to replace {label} `{}` from `{}`",
path.display(),
tmp_path.display()
)
})?;
sync_parent_directory(path, label)
})
})();
if result.is_err() {
let _ = fs::remove_file(&tmp_path);
@@ -48,21 +38,6 @@ pub fn write_file_atomically(path: &Path, bytes: &[u8], label: &str) -> Result<(
result
}
fn sync_parent_directory(path: &Path, label: &str) -> Result<()> {
let parent = path
.parent()
.filter(|parent| !parent.as_os_str().is_empty())
.unwrap_or_else(|| Path::new("."));
fs::File::open(parent)
.and_then(|directory| directory.sync_all())
.with_context(|| {
format!(
"failed to sync {label} parent directory `{}`",
parent.display()
)
})
}
fn unique_temp_path(path: &Path) -> PathBuf {
let nonce = SystemTime::now()
.duration_since(UNIX_EPOCH)
@@ -89,7 +64,7 @@ mod tests {
use anyhow::Result;
use super::write_file_atomically;
use super::write_profile_file;
struct TempDir {
path: PathBuf,
@@ -102,7 +77,7 @@ mod tests {
.expect("system clock should be after epoch")
.as_nanos();
let path = std::env::temp_dir().join(format!(
"moli-atomic-write-{name}-{}-{nonce}",
"moli-profile-file-{name}-{}-{nonce}",
std::process::id()
));
Self { path }
@@ -116,7 +91,22 @@ mod tests {
}
#[test]
fn atomic_write_uses_unique_temp_name_instead_of_fixed_tmp_path() -> Result<()> {
fn profile_file_creates_and_overwrites_file_in_new_directory() -> Result<()> {
let temp = TempDir::new("create-replace");
let parent = temp.path.join("profile with spaces \u{914d}\u{7f6e}");
let target = parent.join("profile.json");
write_profile_file(&target, b"initial profile", "profile test")?;
assert_eq!(fs::read(&target)?, b"initial profile");
write_profile_file(&target, b"updated", "profile test")?;
assert_eq!(fs::read(&target)?, b"updated");
assert_eq!(fs::read_dir(&parent)?.count(), 1);
Ok(())
}
#[test]
fn profile_file_uses_unique_temp_name_instead_of_fixed_tmp_path() -> Result<()> {
let temp = TempDir::new("unique");
let target = temp.path.join("profile.json");
let mut fixed_tmp = target.as_os_str().to_owned();
@@ -126,7 +116,7 @@ mod tests {
fs::write(&target, b"old profile")?;
fs::write(&fixed_tmp, b"stale fixed tmp")?;
write_file_atomically(&target, b"new profile", "profile test")?;
write_profile_file(&target, b"new profile", "profile test")?;
assert_eq!(fs::read(&target)?, b"new profile");
assert_eq!(fs::read(&fixed_tmp)?, b"stale fixed tmp");
@@ -134,18 +124,19 @@ mod tests {
}
#[test]
fn atomic_write_removes_unique_temp_after_replace_error() -> Result<()> {
fn profile_file_removes_unique_temp_after_replace_error() -> Result<()> {
let temp = TempDir::new("replace-error");
let target = temp.path.join("profile.json");
fs::create_dir_all(&target)?;
assert!(write_file_atomically(&target, b"new profile", "profile test").is_err());
let error = write_profile_file(&target, b"new profile", "profile test")
.expect_err("replacing a directory should fail");
let generated_temps = fs::read_dir(&temp.path)?
.filter_map(|entry| entry.ok())
.filter(|entry| entry.file_type().is_ok_and(|file_type| file_type.is_file()))
.count();
assert_eq!(generated_temps, 0);
assert!(error.to_string().contains("failed to replace profile test"));
assert!(error.to_string().contains(&target.display().to_string()));
assert!(error.downcast_ref::<std::io::Error>().is_some());
assert_eq!(fs::read_dir(&temp.path)?.count(), 1);
assert!(target.is_dir());
Ok(())
}
}
+2 -2
View File
@@ -4,7 +4,7 @@ use anyhow::{Context, Result, bail};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use crate::{BrowserProfilePaths, DEFAULT_PROFILE_PARTITION_ID, write_file_atomically};
use crate::{BrowserProfilePaths, DEFAULT_PROFILE_PARTITION_ID, write_profile_file};
pub const PROFILE_MANIFEST_VERSION: u32 = 5;
@@ -166,7 +166,7 @@ fn save_profile_manifest(
) -> Result<()> {
let bytes =
serde_json::to_vec_pretty(manifest).context("failed to serialize profile manifest")?;
write_file_atomically(&paths.manifest_path, &bytes, "profile manifest")
write_profile_file(&paths.manifest_path, &bytes, "profile manifest")
}
fn validate_profile_manifest(
+1 -1
View File
@@ -161,7 +161,7 @@ pub fn import_session_state(request: &ImportRequest<'_>) -> Result<ImportSummary
};
let storage = storage::prepare(partition.local_storage_path(), imported_storage)?;
if let Some(bytes) = storage {
moli_browser_profile::write_file_atomically(
moli_browser_profile::write_profile_file(
partition.local_storage_path(),
&bytes,
"localStorage import",
@@ -910,7 +910,7 @@ fn persist_json_web_storage(path: &Path, areas: &SerializedWebStorageAreas) -> R
let bytes =
serde_json::to_vec_pretty(&file).context("failed to serialize localStorage store")?;
moli_browser_profile::write_file_atomically(path, &bytes, "localStorage store")
moli_browser_profile::write_profile_file(path, &bytes, "localStorage store")
}
#[cfg(test)]
@@ -318,7 +318,7 @@ fn persist_json_service_worker_resource_store(
};
let bytes = serde_json::to_vec_pretty(&file)
.context("failed to serialize Service Worker resource store")?;
moli_browser_profile::write_file_atomically(path, &bytes, "Service Worker resource store")
moli_browser_profile::write_profile_file(path, &bytes, "Service Worker resource store")
}
#[derive(Debug, Serialize, Deserialize)]
+2 -2
View File
@@ -2145,7 +2145,7 @@ fn save_storage_bucket_cache_file(
};
let bytes = serde_json::to_vec_pretty(&json)
.context("failed to serialize StorageBucket CacheStorage")?;
moli_browser_profile::write_file_atomically(path, &bytes, "StorageBucket CacheStorage file")
moli_browser_profile::write_profile_file(path, &bytes, "StorageBucket CacheStorage file")
}
impl JsonStorageBucketBackend {
@@ -2285,7 +2285,7 @@ impl JsonStorageBucketBackend {
};
let bytes =
serde_json::to_vec_pretty(&json).context("failed to serialize storage bucket store")?;
moli_browser_profile::write_file_atomically(&self.path, &bytes, "storage bucket store")
moli_browser_profile::write_profile_file(&self.path, &bytes, "storage bucket store")
}
fn migrate_legacy_implicit_default_cache_storage(&mut self) -> bool {