From d5afeb608874b7f11daaab8df8f0329901b791da Mon Sep 17 00:00:00 2001
From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com>
Date: Tue, 23 Jun 2026 12:38:02 -0700
Subject: [PATCH] io: add platform-aware fsync helpers (dir + durable file
 sync)

Add an io::fsync module with a per-OS split (posix/windows) mirroring the
writeback_file convention, replacing two duplicated dir-fsync copies:

- dir(): POSIX directory fsync; a no-op on Windows, where std cannot open
  a directory as a File and the failed open logged a spurious warning on
  every mapfile write.
- file_durable(): opens the target read+write before sync_all so the flush
  succeeds on Windows, where FlushFileBuffers rejects a read-only handle
  with ERROR_ACCESS_DENIED.

Point the mapfile writer at the shared dir() helper.
---
 src/disc/mapfile.rs     | 24 +----------
 src/io/fsync/mod.rs     | 92 +++++++++++++++++++++++++++++++++++++++++
 src/io/fsync/posix.rs   | 18 ++++++++
 src/io/fsync/windows.rs | 13 ++++++
 src/io/mod.rs           |  1 +
 5 files changed, 126 insertions(+), 22 deletions(-)
 create mode 100644 src/io/fsync/mod.rs
 create mode 100644 src/io/fsync/posix.rs
 create mode 100644 src/io/fsync/windows.rs

diff --git a/src/disc/mapfile.rs b/src/disc/mapfile.rs
index b0a9383..9fe0cc6 100644
--- a/src/disc/mapfile.rs
+++ b/src/disc/mapfile.rs
@@ -630,32 +630,12 @@ impl Mapfile {
         // this window is the wide one. Best-effort: a dir that can't be
         // opened/synced (some filesystems, Windows) is not a write failure.
         if let Some(parent) = self.path.parent() {
-            fsync_dir(parent);
+            crate::io::fsync::dir(parent);
         }
         Ok(())
     }
 }
 
-/// fsync a directory so a prior `rename(2)` into it is durable. After a
-/// crash a renamed file's dirent can otherwise be lost even though the
-/// rename returned, because it is still page-cache-only. Best-effort:
-/// opening the directory for read and `sync_all()`ing it is the POSIX way
-/// to flush its metadata; failures (unsupported fs, Windows) are logged
-/// and ignored rather than propagated, since the file bytes are already
-/// durable and the caller's write succeeded.
-fn fsync_dir(dir: &Path) {
-    match std::fs::File::open(dir) {
-        Ok(f) => {
-            if let Err(e) = f.sync_all() {
-                tracing::warn!(path = %dir.display(), error = %e, "failed to fsync mapfile directory");
-            }
-        }
-        Err(e) => {
-            tracing::warn!(path = %dir.display(), error = %e, "could not open mapfile directory to fsync");
-        }
-    }
-}
-
 impl Drop for Mapfile {
     /// Best-effort flush on drop so a sweep / patch that returns early
     /// (or unwinds) doesn't lose its in-memory state. Errors here are
@@ -863,7 +843,7 @@ mod tests {
 
         // The directly-called dir fsync helper must be a no-op-on-error,
         // never a panic, even for a nonexistent directory.
-        fsync_dir(&dir.join("does-not-exist"));
+        crate::io::fsync::dir(&dir.join("does-not-exist"));
 
         let loaded = Mapfile::load(&p).unwrap();
         assert_eq!(loaded.entries(), mf.entries());
diff --git a/src/io/fsync/mod.rs b/src/io/fsync/mod.rs
new file mode 100644
index 0000000..9b0aca0
--- /dev/null
+++ b/src/io/fsync/mod.rs
@@ -0,0 +1,92 @@
+//! Platform-aware crash-durability primitives.
+//!
+//! Two flush operations need OS-specific handling to make a write survive a
+//! crash / power loss:
+//!
+//! - [`dir`] — fsync a directory so a prior `rename(2)` into it is durable.
+//!   After a crash a renamed file's dirent can otherwise be lost even though
+//!   the rename returned, because it is still page-cache-only. This is a POSIX
+//!   concept: on Windows std cannot even open a directory as a `File` (it does
+//!   not set `FILE_FLAG_BACKUP_SEMANTICS`), and NTFS/ReFS commit the rename's
+//!   dirent without an explicit directory flush — so it is a no-op there
+//!   rather than a failed open that logs on every marker write.
+//!
+//! - [`file_durable`] — fsync a file's contents + metadata. Opens the file
+//!   **read+write**: on Windows `File::sync_all` maps to `FlushFileBuffers`,
+//!   which requires a handle with write access and returns
+//!   `ERROR_ACCESS_DENIED` (os error 5) on a read-only handle. (A read-only
+//!   `File::open` + `sync_all` is legal on POSIX, which is why that bug only
+//!   bit Windows.) The open mode is platform-uniform, so this lives here with
+//!   no dispatch.
+//!
+//! Per the crate convention (see [`crate::io::writeback_file`]), platform
+//! dispatch happens once here via cfg-gated `mod` decls — callers carry no
+//! inline `#[cfg(...)]`.
+
+use std::io;
+use std::path::Path;
+
+#[cfg(not(windows))]
+mod posix;
+#[cfg(windows)]
+mod windows;
+
+#[cfg(not(windows))]
+use posix as platform;
+#[cfg(windows)]
+use windows as platform;
+
+/// fsync a directory so a prior `rename(2)` into it is durable. Best-effort:
+/// failures are logged and swallowed, never propagated — the renamed file's
+/// bytes are already synced and the caller's write itself succeeded. No-op on
+/// Windows (see module docs).
+pub fn dir(path: &Path) {
+    platform::fsync_dir(path)
+}
+
+/// Durably flush an existing file's contents + metadata to stable storage.
+///
+/// Opens the file read+write (not read-only) so the flush succeeds on every
+/// platform — see the module docs for the Windows `FlushFileBuffers` rationale.
+/// The file must already exist; its bytes are left intact (no create/truncate).
+pub fn file_durable(path: &Path) -> io::Result<()> {
+    let f = std::fs::OpenOptions::new()
+        .read(true)
+        .write(true)
+        .open(path)?;
+    f.sync_all()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// `file_durable` opens read+write (so the flush works on Windows) and
+    /// syncs an existing file; a missing path surfaces as `Err` so the caller
+    /// treats it as "not durably synced". Platform-uniform — same on
+    /// unix/windows.
+    #[test]
+    fn file_durable_ok_for_existing_err_for_missing() {
+        let td = tempfile::tempdir().unwrap();
+        let f = td.path().join("data.bin");
+        std::fs::write(&f, b"durable").unwrap();
+        assert!(
+            file_durable(&f).is_ok(),
+            "an existing file must open read+write and fsync cleanly"
+        );
+        assert!(
+            file_durable(&td.path().join("absent.bin")).is_err(),
+            "a missing file must surface the open failure as Err"
+        );
+    }
+
+    /// `dir` is best-effort: it must return normally for a real directory
+    /// (POSIX fsyncs it, Windows no-ops) and must swallow — never panic on —
+    /// a missing directory.
+    #[test]
+    fn dir_is_best_effort_never_panics() {
+        let td = tempfile::tempdir().unwrap();
+        dir(td.path());
+        dir(&td.path().join("does-not-exist"));
+    }
+}
diff --git a/src/io/fsync/posix.rs b/src/io/fsync/posix.rs
new file mode 100644
index 0000000..950c35a
--- /dev/null
+++ b/src/io/fsync/posix.rs
@@ -0,0 +1,18 @@
+//! POSIX directory-fsync. Active on unix and any non-Windows fallback target
+//! (BSD, illumos, …) — all share the same `File::open(dir).sync_all()`
+//! semantics. The Windows no-op lives in the sibling `windows` module.
+
+use std::path::Path;
+
+pub(super) fn fsync_dir(dir: &Path) {
+    match std::fs::File::open(dir) {
+        Ok(f) => {
+            if let Err(e) = f.sync_all() {
+                tracing::warn!(path = %dir.display(), error = %e, "failed to fsync directory");
+            }
+        }
+        Err(e) => {
+            tracing::warn!(path = %dir.display(), error = %e, "could not open directory to fsync");
+        }
+    }
+}
diff --git a/src/io/fsync/windows.rs b/src/io/fsync/windows.rs
new file mode 100644
index 0000000..e726f20
--- /dev/null
+++ b/src/io/fsync/windows.rs
@@ -0,0 +1,13 @@
+//! Windows directory-fsync: a no-op.
+//!
+//! Directory fsync is a POSIX concept. std cannot open a directory as a `File`
+//! on Windows (it does not set `FILE_FLAG_BACKUP_SEMANTICS`), so the POSIX impl
+//! could only ever fail the open and log a spurious warning on every marker /
+//! mapfile write. NTFS/ReFS commit a rename's directory entry without an
+//! explicit directory flush, so skipping it here is correct — not a durability
+//! regression. (File-content durability is handled platform-uniformly by
+//! [`super::file_durable`].)
+
+use std::path::Path;
+
+pub(super) fn fsync_dir(_dir: &Path) {}
diff --git a/src/io/mod.rs b/src/io/mod.rs
index 1d4701f..98caee6 100644
--- a/src/io/mod.rs
+++ b/src/io/mod.rs
@@ -30,6 +30,7 @@
 pub(crate) mod bounded;
 pub mod byte_prefetcher;
 pub mod file_sector_source;
+pub mod fsync;
 pub mod sink;
 mod writeback;
 mod writeback_file;