Skip to content

Commit aea4aa0

Browse files
committed
Data safety: Make config JSON writes power-loss durable
The four config-store persistence helpers (`manual_servers.rs`, `known_shares.rs`, `search/history.rs`, `selection/history.rs`) did `fs::write(tmp) + fs::rename(tmp, path)` with no fsync. That's atomic against process death but NOT against power loss: the rename can land in the journal while the temp's data blocks are still only in the page cache, leaving the destination zero-length or torn. `manual-servers.json` is the one that matters most: it holds user-entered SMB servers that aren't rediscoverable via mDNS. - Extracted a shared `config::durable_write_json(path, tmp, content)` that writes the temp, `sync_all()`s it before the rename, then best-effort fsyncs the parent directory so the rename itself is durable. Mirrors `LocalPosixVolume::write_from_stream`'s data-loss-class discipline. - All four `atomic_write_json` wrappers now delegate to it, keeping their existing `.json.tmp` temp naming (so each store's `cleanup_tmp_file` stale-temp recovery still works). Updated their doc comments to describe the real durability guarantee instead of just "atomic". - Added round-trip + overwrite unit tests for the shared helper; the existing `manual_servers` concurrent/sequential write tests already exercise the new path end to end.
1 parent 0a154f2 commit aea4aa0

5 files changed

Lines changed: 85 additions & 17 deletions

File tree

apps/desktop/src-tauri/src/config.rs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//!
33
//! These can be extracted to environment variables or a config file in the future.
44
5-
use std::path::PathBuf;
5+
use std::io::Write;
6+
use std::path::{Path, PathBuf};
67
use tauri::{AppHandle, Manager, Runtime};
78

89
/// Icon size in pixels (32x32 for retina display)
@@ -58,6 +59,47 @@ pub fn log_app_data_dir<R: Runtime>(app: &AppHandle<R>) {
5859
}
5960
}
6061

62+
/// Writes `content` to `path` durably: write the bytes to `tmp`, fsync the temp file, rename
63+
/// it over `path`, then fsync the parent directory so the rename itself survives a power loss.
64+
///
65+
/// `fs::write(tmp) + fs::rename(tmp, path)` alone is atomic against *process* death (the rename
66+
/// swaps the directory entry as a unit) but NOT against a power loss / hard crash: the rename can
67+
/// land in the filesystem journal while the temp's data blocks are still only in the page cache,
68+
/// leaving the destination zero-length or torn. The two fsyncs close that window. This mirrors the
69+
/// data-loss-class write discipline in `LocalPosixVolume::write_from_stream` (the
70+
/// copy/move path that survives eject/sleep/power-loss).
71+
///
72+
/// The caller owns the temp-path convention (it picks `tmp`) so each store keeps its existing
73+
/// `cleanup_tmp_file` stale-temp recovery. The parent-directory fsync is best-effort: some
74+
/// filesystems reject opening a directory for fsync, so a failure there is logged and ignored
75+
/// rather than failing the whole write (the file's data is already durable at that point).
76+
pub fn durable_write_json(path: &Path, tmp: &Path, content: &str) -> std::io::Result<()> {
77+
{
78+
let mut file = std::fs::File::create(tmp)?;
79+
file.write_all(content.as_bytes())?;
80+
// fsync the temp's data + metadata before the rename so the bytes are on disk, not just
81+
// in the page cache, when the directory entry swaps.
82+
file.sync_all()?;
83+
}
84+
85+
std::fs::rename(tmp, path)?;
86+
87+
// Best-effort: fsync the parent directory so the rename (the new directory entry) is durable
88+
// too. Logged and ignored on failure, matching `LocalPosixVolume`'s parent-dir fsync.
89+
if let Some(parent) = path.parent() {
90+
match std::fs::File::open(parent).and_then(|dir| dir.sync_all()) {
91+
Ok(()) => {}
92+
Err(e) => log::debug!(
93+
target: "write_durability",
94+
"durable_write_json: parent dir fsync skipped for {}: {e}",
95+
parent.display()
96+
),
97+
}
98+
}
99+
100+
Ok(())
101+
}
102+
61103
// MCP Server Security Design:
62104
// --------------------------
63105
// The MCP (Model Context Protocol) bridge allows AI assistants to control the app.
@@ -93,4 +135,30 @@ mod tests {
93135
// Falling through to the Tauri default is the documented behavior.
94136
assert_eq!(data_dir_from_env(Some("")), None);
95137
}
138+
139+
#[test]
140+
fn durable_write_json_round_trips_content() {
141+
let dir = tempfile::tempdir().expect("create temp dir");
142+
let path = dir.path().join("store.json");
143+
let tmp = path.with_extension("json.tmp");
144+
145+
durable_write_json(&path, &tmp, r#"{"a":1}"#).expect("durable write");
146+
147+
let read_back = std::fs::read_to_string(&path).expect("read back");
148+
assert_eq!(read_back, r#"{"a":1}"#);
149+
// The temp file must be consumed by the rename, not left behind.
150+
assert!(!tmp.exists(), "temp file should be renamed away, not left behind");
151+
}
152+
153+
#[test]
154+
fn durable_write_json_overwrites_existing_file() {
155+
let dir = tempfile::tempdir().expect("create temp dir");
156+
let path = dir.path().join("store.json");
157+
let tmp = path.with_extension("json.tmp");
158+
159+
std::fs::write(&path, "old contents that are much longer").expect("seed file");
160+
durable_write_json(&path, &tmp, "new").expect("durable write");
161+
162+
assert_eq!(std::fs::read_to_string(&path).expect("read back"), "new");
163+
}
96164
}

apps/desktop/src-tauri/src/network/known_shares.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,12 @@ fn get_known_shares_mutex() -> &'static Mutex<KnownSharesStore> {
5757
KNOWN_SHARES.get_or_init(|| Mutex::new(KnownSharesStore::default()))
5858
}
5959

60-
/// Atomically writes content to a file using write-to-temp + rename.
61-
/// On failure, the original file (if any) remains intact.
60+
/// Durably writes content to a file using write-to-temp + fsync + rename + parent-dir fsync.
61+
/// On failure, the original file (if any) remains intact. The fsyncs make the write survive a
62+
/// power loss, not just process death. See `crate::config::durable_write_json` for the rationale.
6263
fn atomic_write_json(path: &Path, content: &str) -> std::io::Result<()> {
6364
let tmp = path.with_extension("json.tmp");
64-
fs::write(&tmp, content)?;
65-
fs::rename(&tmp, path)?;
66-
Ok(())
65+
crate::config::durable_write_json(path, &tmp, content)
6766
}
6867

6968
/// Removes a stale `.tmp` file left over from a crash during atomic write.

apps/desktop/src-tauri/src/network/manual_servers.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,14 @@ pub async fn check_reachability(host: &str, port: u16) -> Result<(), String> {
362362
// Atomic file writes
363363
// ---------------------------------------------------------------------------
364364

365-
/// Atomically writes content to a file using write-to-temp + rename.
366-
/// On failure, the original file (if any) remains intact.
365+
/// Durably writes content to a file using write-to-temp + fsync + rename + parent-dir fsync.
366+
/// On failure, the original file (if any) remains intact. The fsyncs make the write survive a
367+
/// power loss, not just process death: `manual-servers.json` holds user-entered SMB servers that
368+
/// aren't rediscoverable via mDNS, so a torn / zero-length write here loses real config. See
369+
/// `crate::config::durable_write_json` for the durability rationale.
367370
fn atomic_write_json(path: &Path, content: &str) -> std::io::Result<()> {
368371
let tmp = path.with_extension("json.tmp");
369-
fs::write(&tmp, content)?;
370-
fs::rename(&tmp, path)?;
371-
Ok(())
372+
crate::config::durable_write_json(path, &tmp, content)
372373
}
373374

374375
/// Removes a stale `.tmp` file left over from a crash during atomic write.

apps/desktop/src-tauri/src/search/history.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,11 @@ fn canonical_key(entry: &HistoryEntry) -> String {
168168
// Atomic file I/O helpers (mirrors `network/known_shares.rs` and `manual_servers.rs`).
169169
// ---------------------------------------------------------------------------
170170

171+
/// Durably writes content to a file: write-to-temp + fsync + rename + parent-dir fsync, so the
172+
/// write survives a power loss, not just process death. See `crate::config::durable_write_json`.
171173
fn atomic_write_json(path: &Path, content: &str) -> std::io::Result<()> {
172174
let tmp = path.with_extension("json.tmp");
173-
fs::write(&tmp, content)?;
174-
fs::rename(&tmp, path)?;
175-
Ok(())
175+
crate::config::durable_write_json(path, &tmp, content)
176176
}
177177

178178
fn cleanup_tmp_file(path: &Path) {

apps/desktop/src-tauri/src/selection/history.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ fn canonical_key(entry: &SelectionHistoryEntry) -> String {
142142
// Atomic file I/O helpers (mirrors `crate::search::history`).
143143
// ---------------------------------------------------------------------------
144144

145+
/// Durably writes content to a file: write-to-temp + fsync + rename + parent-dir fsync, so the
146+
/// write survives a power loss, not just process death. See `crate::config::durable_write_json`.
145147
fn atomic_write_json(path: &Path, content: &str) -> std::io::Result<()> {
146148
let tmp = path.with_extension("json.tmp");
147-
fs::write(&tmp, content)?;
148-
fs::rename(&tmp, path)?;
149-
Ok(())
149+
crate::config::durable_write_json(path, &tmp, content)
150150
}
151151

152152
fn cleanup_tmp_file(path: &Path) {

0 commit comments

Comments
 (0)