shifathosn
/
photocraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Atomic saves: one crash-safe atomic_write for every document write (#230) (#230)

This commit is contained in:
Brandon Thomas 2026-10-05 11:16:13 -04:00 committed by GitHub
parent eb11e1af46
commit 82270711de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 522 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
apps/photocraft/src/services.rs
View File

@ -67,14 +67,11 @@ fn recovery_dir() -> Option<PathBuf> {
config_dir().map(|d| d.join("Recovery"))
}
/// Write `bytes` atomically (temp file + rename) so a crash never leaves half a preferences file.
/// Write `bytes` crash-safely (temp file beside the target, fsync, rename, directory fsync; see
/// [`photocraft_format::atomic`]). Every document write (Save, Save As, Export, Save for Web) and
/// the preferences go through here, so a failed or interrupted save never destroys the old file.
fn write_atomic(path: &Path, bytes: &[u8]) -> Result<(), String> {
if let Some(dir) = path.parent() {
std::fs::create_dir_all(dir).map_err(|e| e.to_string())?;
}
let tmp = path.with_extension("tmp");
std::fs::write(&tmp, bytes).map_err(|e| e.to_string())?;
std::fs::rename(&tmp, path).map_err(|e| e.to_string())
photocraft_format::atomic_write(path, bytes).map_err(|e| e.to_string())
}
pub fn native(automation: Option<photocraft_automation::AuthorizedWorkspace>) -> Services {
@ -123,7 +120,7 @@ pub fn native(automation: Option<photocraft_automation::AuthorizedWorkspace>) ->
}
Some(d.save_file()?.to_string_lossy().to_string())
})),
write: Some(Box::new(|path: &str, bytes: &[u8]| std::fs::write(path, bytes).map_err(|e| e.to_string()))),
write: Some(Box::new(|path: &str, bytes: &[u8]| write_atomic(Path::new(path), bytes))),
automation_read,
automation_write,
automation_command,

6
crates/automation/src/files.rs
View File

@ -61,10 +61,8 @@ pub fn save(
return Ok(Vec::new());
}
let r = photocraft_io::export(doc, &ext, opts)?;
if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) {
std::fs::create_dir_all(parent).map_err(|e| AutomationError::Io(e.to_string()))?;
}
std::fs::write(path, &r.bytes).map_err(|e| AutomationError::Io(format!("{}: {e}", path.display())))?;
// Crash-safe: a failed write never destroys the previous file.
photocraft_format::atomic_write(path, &r.bytes).map_err(|e| AutomationError::Io(e.to_string()))?;
Ok(r.warnings)
}

2
crates/automation/src/headless.rs
View File

@ -127,7 +127,7 @@ impl Headless {
let requested = path.to_str().ok_or_else(|| AutomationError::BadRequest("automation paths must be valid UTF-8".into()))?;
match &self.filesystem {
Filesystem::Denied => Err(AutomationError::BadRequest("automation filesystem access is not granted: write authority is absent".into())),
Filesystem::TrustedLocal => std::fs::write(path, bytes).map_err(|error| AutomationError::Io(format!("{}: {error}", path.display()))),
Filesystem::TrustedLocal => photocraft_format::atomic_write(path, bytes).map_err(|error| AutomationError::Io(error.to_string())),
Filesystem::Workspace(workspace) => workspace.write(requested, bytes),
}
}

69
crates/automation/src/workspace.rs
View File

@ -6,6 +6,7 @@ use std::sync::Arc;
use cap_std::ambient_authority;
use cap_std::fs::{Dir, OpenOptions};
use photocraft_format::atomic::RenameRetry;
use serde_json::Value;
use crate::AutomationError;
@ -50,7 +51,10 @@ impl AuthorizedWorkspace {
Ok(bytes)
}
/// Create or replace one file below the configured write root.
/// Create or replace one file below the configured write root, crash-safely: the bytes go to
/// a temporary file beside the target, which is synced and renamed over it (the same steps
/// as [`photocraft_format::atomic_write`], through the directory capability). On failure
/// the previous file is untouched and the temporary file is removed.
///
/// The parent directory must already exist. `cap-std` performs path
/// resolution and file creation relative to the held directory handle, so
@ -58,11 +62,29 @@ impl AuthorizedWorkspace {
pub fn write(&self, path: &str, bytes: &[u8]) -> Result<(), AutomationError> {
let relative = relative_path(path)?;
let root = self.write.as_ref().ok_or_else(|| AutomationError::BadRequest(format!("{DENIED}: write authority is absent")))?;
let leaf = relative.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let parent = relative.parent().map(Path::to_path_buf).unwrap_or_default();
let tmp = parent.join(photocraft_format::atomic::temp_name(&leaf));
let mut options = OpenOptions::new();
options.write(true).create(true).truncate(true);
let mut file = root.dir.open_with(&relative, &options).map_err(|e| file_error("write", path, e))?;
file.write_all(bytes).map_err(|e| file_error("write", path, e))?;
file.flush().map_err(|e| file_error("write", path, e))
options.write(true).create_new(true);
let written = root.dir.open_with(&tmp, &options).and_then(|mut file| {
file.write_all(bytes)?;
file.sync_all()
});
let renamed = written.and_then(|()| photocraft_format::atomic::retry_rename(RenameRetry::platform(), || root.dir.rename(&tmp, &root.dir, &relative)));
if let Err(e) = renamed {
let _ = root.dir.remove_file(&tmp);
return Err(file_error("write", path, e));
}
// Flush the directory entry (Unix); best effort, the new bytes are already in place.
#[cfg(unix)]
{
let dir = if parent.as_os_str().is_empty() { PathBuf::from(".") } else { parent };
if let Ok(d) = root.dir.open(&dir) {
let _ = d.sync_all();
}
}
Ok(())
}
}
@ -256,6 +278,43 @@ mod tests {
assert_eq!(std::fs::read(inside.join("new-output.txt")).unwrap(), b"created");
}
fn temp_files(dir: &Path) -> Vec<String> {
std::fs::read_dir(dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().into_owned()).filter(|n| n.ends_with(".tmp")).collect()
}
#[test]
fn write_replaces_atomically_and_leaves_no_temp_file() {
let (inside, _, workspace) = roots("atomic");
std::fs::create_dir_all(inside.join("sub")).unwrap();
workspace.write("sub/out.psd", b"original").unwrap();
workspace.write("sub/out.psd", b"replacement").unwrap();
assert_eq!(std::fs::read(inside.join("sub/out.psd")).unwrap(), b"replacement");
assert!(temp_files(&inside.join("sub")).is_empty());
// Renaming over a directory fails: nothing is left behind and the directory survives.
std::fs::create_dir_all(inside.join("adir")).unwrap();
assert!(workspace.write("adir", b"x").is_err());
assert!(inside.join("adir").is_dir());
assert!(temp_files(&inside).is_empty());
}
#[cfg(unix)]
#[test]
fn write_into_read_only_folder_keeps_the_original() {
use std::os::unix::fs::PermissionsExt;
let (inside, _, workspace) = roots("readonly");
let ro = inside.join("ro");
std::fs::create_dir_all(&ro).unwrap();
std::fs::write(ro.join("doc.psd"), b"original").unwrap();
std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o555)).unwrap();
let root_user = std::fs::File::create(ro.join("probe")).is_ok();
if !root_user {
assert!(workspace.write("ro/doc.psd", b"new").is_err());
assert_eq!(std::fs::read(ro.join("doc.psd")).unwrap(), b"original");
assert!(temp_files(&ro).is_empty());
}
std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o755)).unwrap();
}
#[test]
fn rejects_absolute_traversal_mixed_prefix_and_malformed_paths_without_panicking() {
let (_, _, workspace) = roots("reject");

2
crates/engine/src/automate_cmds.rs
View File

@ -307,7 +307,7 @@ fn write_shim(droplet: &str) -> Result<String> {
"#!/bin/sh\n# PhotoCraft droplet: runs the action on the files given (or dropped).\nexec \"${{PHOTOCRAFT_CLI:-photocraft-cli}}\" droplet \"{}\" \"$@\"\n",
abs.replace('"', "\\\"")
);
std::fs::write(&shim, body).map_err(|e| EngineError::Other(format!("{shim}: {e}")))?;
crate::file_cmds::write_file(&shim, body.as_bytes())?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;

7
crates/engine/src/file_cmds.rs
View File

@ -84,11 +84,10 @@ pub(crate) fn read_file(path: &str) -> Result<Vec<u8>> {
}
#[cfg(not(target_arch = "wasm32"))]
/// Every file the engine writes goes through here: crash-safe (temp file + fsync + rename, see
/// [`photocraft_format::atomic`]), so a failed save never destroys the previous file.
pub(crate) fn write_file(path: &str, bytes: &[u8]) -> Result<()> {
if let Some(dir) = std::path::Path::new(path).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?;
}
std::fs::write(path, bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))
photocraft_format::atomic_write(std::path::Path::new(path), bytes).map_err(|e| EngineError::Other(e.to_string()))
}
#[cfg(target_arch = "wasm32")]
pub(crate) fn write_file(path: &str, _bytes: &[u8]) -> Result<()> {

2
crates/engine/src/layer_menu_cmds.rs
View File

@ -709,7 +709,7 @@ fn export_layer(s: &mut Session, p: &Value, cmd: &str, png_only: bool) -> Result
#[cfg(not(target_arch = "wasm32"))]
fn write_file(path: &str, bytes: &[u8]) -> Result<()> {
std::fs::write(path, bytes).map_err(|e| other(format!("{path}: {e}")))
crate::file_cmds::write_file(path, bytes)
}
#[cfg(target_arch = "wasm32")]

2
crates/engine/src/pattern_cmds.rs
View File

@ -288,7 +288,7 @@ fn export(s: &mut Session, p: &Value) -> Result<Value> {
None => s.patterns.items.clone(),
};
let bytes = photocraft_io::pattern_map::write_pat(&pats).map_err(EngineError::Other)?;
std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::file_cmds::write_file(path, &bytes)?;
Ok(json!({"path": path, "count": pats.len(), "bytes": bytes.len()}))
}

8
crates/engine/src/preset_store.rs
View File

@ -183,13 +183,7 @@ impl PresetBackend for DirBackend {
let path = self.path(name)?;
let dir = path.parent().ok_or_else(|| format!("{name}: no parent directory"))?;
std::fs::create_dir_all(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
let leaf = path.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default();
let tmp = dir.join(format!(".{leaf}.tmp"));
std::fs::write(&tmp, bytes).map_err(|e| format!("{name}: {e}"))?;
std::fs::rename(&tmp, &path).map_err(|e| {
let _ = std::fs::remove_file(&tmp);
format!("{name}: {e}")
})
photocraft_format::atomic_write(&path, bytes).map_err(|e| format!("{name}: {e}"))
}
fn remove(&self, name: &str) -> Result<(), String> {
match std::fs::remove_file(self.path(name)?) {

4
crates/engine/src/smart_cmds.rs
View File

@ -605,7 +605,7 @@ fn export_contents(s: &mut Session, p: &Value) -> Result<Value> {
let id = layer_param(s, p)?;
let st = s.active().ok_or(EngineError::NoDocument)?;
let (name, bytes) = source_bytes(&st.doc.metadata, &smart(&st.doc, id)?.source).ok_or_else(|| other("the smart object's contents are unavailable"))?;
std::fs::write(path, &*bytes).map_err(|e| other(format!("can't write {path}: {e}")))?;
crate::file_cmds::write_file(path, &bytes)?;
Ok(json!({"path": path, "fileName": name, "bytes": bytes.len()}))
}
@ -874,7 +874,7 @@ pub fn specs() -> Vec<CommandSpec> {
let path = path_param("layer.smartObjects.convertToLinked", p)?.to_string();
set_source(s, p, "Convert to Linked", false, |meta, src| {
let (_, bytes) = source_bytes(meta, src).ok_or_else(|| other("the smart object's contents are unavailable"))?;
std::fs::write(&path, &*bytes).map_err(|e| other(format!("can't write {path}: {e}")))?;
crate::file_cmds::write_file(&path, &bytes)?;
Ok(SmartSource::Linked { path })
})
}

2
crates/engine/src/variables_cmds.rs
View File

@ -353,7 +353,7 @@ fn export_as_files(s: &mut Session, p: &Value) -> Result<Value> {
let path = format!("{dir}/{stem}.{format}");
let opts = photocraft_io::ExportOptions::default();
let bytes = photocraft_io::export(&doc, format, &opts).map(|r| r.bytes).map_err(|e| EngineError::Other(format!("export `{}`: {e}", set.name)))?;
std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("write `{path}`: {e}")))?;
crate::file_cmds::write_file(&path, &bytes)?;
files.push(path);
}
Ok(json!({"files": files, "count": files.len()}))

4
crates/engine/src/video_cmds.rs
View File

@ -326,7 +326,7 @@ fn render_video(s: &mut Session, p: &Value) -> Result<Value> {
}
let gif = photocraft_codecs::web::encode_gif_animated(w as u32, h as u32, &gframes, true).map_err(|e| EngineError::Other(e.to_string()))?;
let path = format!("{dir}/{stem}.gif");
std::fs::write(&path, &gif).map_err(|e| EngineError::Other(format!("write `{path}`: {e}")))?;
crate::file_cmds::write_file(&path, &gif)?;
return Ok(json!({"frames": frames, "file": path}));
}
@ -341,7 +341,7 @@ fn render_video(s: &mut Session, p: &Value) -> Result<Value> {
sync(&mut doc);
let path = format!("{dir}/{stem}_{f:04}.{format}");
let bytes = photocraft_io::export(&doc, format, &opts).map(|r| r.bytes).map_err(|e| EngineError::Other(format!("render frame {f}: {e}")))?;
std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("write `{path}`: {e}")))?;
crate::file_cmds::write_file(&path, &bytes)?;
files.push(path);
}
Ok(json!({"frames": files.len(), "dir": dir}))

221
crates/format/src/atomic.rs Normal file
View File

@ -0,0 +1,221 @@
//! Crash-safe file replacement: the one way PhotoCraft writes a document to disk.
//!
//! [`atomic_write`] never truncates the destination. It writes a temporary file in the **same
//! directory**, flushes it to stable storage (`sync_all`), renames it over the destination and,
//! on Unix, fsyncs the directory so the rename itself survives a power loss. If anything fails,
//! the destination keeps its previous bytes and the temporary file is removed.
//!
//! - **Windows:** antivirus scanners and the search indexer briefly hold files open, which makes
//! the replacing rename fail with `PermissionDenied`. The rename is retried with exponential
//! backoff before giving up. (`std::fs::rename` uses `MoveFileExW(MOVEFILE_REPLACE_EXISTING)`,
//! i.e. replace semantics.)
//! - **Cross-device (`EXDEV`):** cannot happen while the temporary file sits beside the target;
//! if it does anyway, the save fails with a clear error. It never falls back to writing the
//! destination in place.
//! - **Unwritable folder:** the save fails with a clear error instead of writing in place.
//!
//! On the web there is no file system: writes go through the platform services (downloads), so
//! this module is never reached there; on `wasm32` every call returns the `std` "unsupported"
//! error.
//!
//! [`atomic_write_with`] takes an [`AtomicFs`] so tests can inject failures at each step.
use std::io::{self, Write};
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicU64, Ordering};
/// The file-system steps [`atomic_write_with`] performs. [`RealFs`] is the real thing; tests
/// wrap it to inject failures.
pub trait AtomicFs {
/// Create `tmp` (it must not exist), write all of `bytes` and `sync_all` it.
fn write_new(&self, tmp: &Path, bytes: &[u8]) -> io::Result<()>;
/// Rename `from` over `to`, replacing it.
fn rename(&self, from: &Path, to: &Path) -> io::Result<()>;
/// Remove a (temporary) file.
fn remove(&self, path: &Path) -> io::Result<()>;
/// Flush a directory entry change (Unix); a no-op elsewhere.
fn sync_dir(&self, dir: &Path) -> io::Result<()>;
/// Copy the destination's permissions onto the temporary file, so replacing a file keeps its
/// mode. Best effort.
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()>;
}
/// The real file system.
#[derive(Debug, Clone, Copy, Default)]
pub struct RealFs;
impl AtomicFs for RealFs {
fn write_new(&self, tmp: &Path, bytes: &[u8]) -> io::Result<()> {
let mut f = std::fs::OpenOptions::new().write(true).create_new(true).open(tmp)?;
f.write_all(bytes)?;
f.sync_all()
}
fn rename(&self, from: &Path, to: &Path) -> io::Result<()> {
std::fs::rename(from, to)
}
fn remove(&self, path: &Path) -> io::Result<()> {
std::fs::remove_file(path)
}
fn sync_dir(&self, dir: &Path) -> io::Result<()> {
#[cfg(unix)]
{
std::fs::File::open(dir)?.sync_all()
}
#[cfg(not(unix))]
{
let _ = dir;
Ok(())
}
}
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()> {
let perms = std::fs::metadata(from)?.permissions();
std::fs::set_permissions(to, perms)
}
}
/// How hard to retry a rename that fails with `PermissionDenied`.
#[derive(Debug, Clone, Copy)]
pub struct RenameRetry {
/// Retries after the first attempt (0 = no retry).
pub attempts: u32,
/// Delay before the first retry; doubles each time.
pub first_delay_ms: u64,
}
impl RenameRetry {
/// The platform default: retry on Windows (about 1.3 s in total), never elsewhere, where
/// `PermissionDenied` is a real permission problem that waiting won't fix.
pub fn platform() -> Self {
if cfg!(windows) { Self { attempts: 7, first_delay_ms: 10 } } else { Self { attempts: 0, first_delay_ms: 0 } }
}
}
/// Atomically replace (or create) `path` with `bytes`. See the module docs.
///
/// Errors keep their [`io::ErrorKind`] and say what happened and that the original file is
/// unchanged.
pub fn atomic_write(path: &Path, bytes: &[u8]) -> io::Result<()> {
atomic_write_with(&RealFs, RenameRetry::platform(), path, bytes)
}
/// [`atomic_write`] through an explicit file system and retry policy (the failure-injection seam).
pub fn atomic_write_with(fs: &dyn AtomicFs, retry: RenameRetry, path: &Path, bytes: &[u8]) -> io::Result<()> {
let target = resolve_symlink(path);
let name = target.file_name().ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, format!("{}: not a file path", path.display())))?;
let dir = match target.parent() {
Some(d) if !d.as_os_str().is_empty() => d.to_path_buf(),
_ => PathBuf::from("."),
};
std::fs::create_dir_all(&dir).map_err(|e| context(e, format!("cannot create the folder {}", dir.display())))?;
// Create a fresh temporary file next to the target (same directory, so the rename never
// crosses a device). The name is unique per process and call; retry a few on collisions.
let mut tmp = None;
for _ in 0..8 {
let candidate = dir.join(temp_name(&name.to_string_lossy()));
match fs.write_new(&candidate, bytes) {
Ok(()) => {
tmp = Some(candidate);
break;
}
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue,
Err(e) => {
// A partial temp file may exist; the destination was never touched.
let _ = fs.remove(&candidate);
return Err(write_error(e, path, &dir));
}
}
}
let tmp = tmp.ok_or_else(|| io::Error::new(io::ErrorKind::AlreadyExists, format!("{}: could not create a temporary file", dir.display())))?;
// Keep the existing file's mode (e.g. 0600) instead of the process default.
if target.exists() {
let _ = fs.copy_permissions(&target, &tmp);
}
if let Err(e) = retry_rename(retry, || fs.rename(&tmp, &target)) {
let _ = fs.remove(&tmp);
return Err(rename_error(e, path));
}
// The new bytes are in place; a failed directory fsync can't be undone and must not report
// the save as failed, so it is best effort.
let _ = fs.sync_dir(&dir);
Ok(())
}
/// Run `rename`, retrying with backoff while it fails with `PermissionDenied` (see
/// [`RenameRetry`]). For callers that rename through their own handles (capability directories).
pub fn retry_rename(retry: RenameRetry, mut rename: impl FnMut() -> io::Result<()>) -> io::Result<()> {
let mut delay = retry.first_delay_ms;
let mut left = retry.attempts;
loop {
match rename() {
Err(e) if e.kind() == io::ErrorKind::PermissionDenied && left > 0 => {
left -= 1;
sleep_ms(delay);
delay = delay.saturating_mul(2);
}
r => return r,
}
}
}
#[cfg(not(target_arch = "wasm32"))]
fn sleep_ms(ms: u64) {
std::thread::sleep(std::time::Duration::from_millis(ms));
}
#[cfg(target_arch = "wasm32")]
fn sleep_ms(_ms: u64) {}
/// Write through a symlink to the file it points at, instead of replacing the link.
fn resolve_symlink(path: &Path) -> PathBuf {
match std::fs::symlink_metadata(path) {
Ok(m) if m.file_type().is_symlink() => std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()),
_ => path.to_path_buf(),
}
}
/// A unique hidden temporary file name to write beside `file` before renaming over it.
pub fn temp_name(file: &str) -> String {
static COUNTER: AtomicU64 = AtomicU64::new(0);
let n = COUNTER.fetch_add(1, Ordering::Relaxed);
// Keep the name short: a long file name plus the suffix must stay under NAME_MAX.
let stem: String = file.chars().take(64).collect();
format!(".{stem}.{}-{n}.pcsave.tmp", std::process::id())
}
fn context(e: io::Error, msg: String) -> io::Error {
io::Error::new(e.kind(), format!("{msg}: {e}"))
}
const UNCHANGED: &str = "the original file was not changed";
fn write_error(e: io::Error, path: &Path, dir: &Path) -> io::Error {
let msg = match e.kind() {
io::ErrorKind::PermissionDenied | io::ErrorKind::ReadOnlyFilesystem => {
format!("cannot save {}: the folder {} is not writable ({e}); {UNCHANGED}", path.display(), dir.display())
}
io::ErrorKind::StorageFull => format!("cannot save {}: the disk is full ({e}); {UNCHANGED}", path.display()),
_ => format!("cannot save {}: {e}; {UNCHANGED}", path.display()),
};
io::Error::new(e.kind(), msg)
}
fn rename_error(e: io::Error, path: &Path) -> io::Error {
let msg = if is_cross_device(&e) {
format!("cannot save {}: the temporary file is on a different volume than the destination ({e}); {UNCHANGED}", path.display())
} else if e.kind() == io::ErrorKind::PermissionDenied {
format!("cannot save {}: the file is locked or read-only ({e}); {UNCHANGED}", path.display())
} else {
format!("cannot save {}: {e}; {UNCHANGED}", path.display())
};
io::Error::new(e.kind(), msg)
}
fn is_cross_device(e: &io::Error) -> bool {
// EXDEV is 18 on Linux, macOS and the BSDs; ERROR_NOT_SAME_DEVICE is 17 on Windows.
e.kind() == io::ErrorKind::CrossesDevices || (cfg!(unix) && e.raw_os_error() == Some(18)) || (cfg!(windows) && e.raw_os_error() == Some(17))
}
#[cfg(test)]
mod tests;

212
crates/format/src/atomic/tests.rs Normal file
View File

@ -0,0 +1,212 @@
use super::*;
use std::cell::Cell;
struct TempDir(PathBuf);
impl TempDir {
fn new(tag: &str) -> Self {
static N: AtomicU64 = AtomicU64::new(0);
let d = std::env::temp_dir().join(format!("pc-atomic-{tag}-{}-{}", std::process::id(), N.fetch_add(1, Ordering::Relaxed)));
let _ = std::fs::remove_dir_all(&d);
std::fs::create_dir_all(&d).unwrap();
Self(d)
}
}
impl Drop for TempDir {
fn drop(&mut self) {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o755));
}
let _ = std::fs::remove_dir_all(&self.0);
}
}
/// Only the target remains: no temporary file was left behind.
fn assert_no_temp(dir: &Path) {
let leftovers: Vec<_> =
std::fs::read_dir(dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().into_owned()).filter(|n| n.ends_with(".tmp")).collect();
assert!(leftovers.is_empty(), "temporary files left behind: {leftovers:?}");
}
const ORIGINAL: &[u8] = b"the user's precious original document";
/// Real file system with failures injected at chosen steps.
#[derive(Default)]
struct Faulty {
/// Write this many bytes, then fail like a full disk.
fail_write_after: Option<usize>,
/// Fail every rename with this kind.
fail_rename: Option<io::ErrorKind>,
/// Fail the first N renames with PermissionDenied (a transient lock), then succeed.
locked_renames: Cell<u32>,
renames: Cell<u32>,
}
impl AtomicFs for Faulty {
fn write_new(&self, tmp: &Path, bytes: &[u8]) -> io::Result<()> {
if let Some(n) = self.fail_write_after {
let mut f = std::fs::OpenOptions::new().write(true).create_new(true).open(tmp)?;
f.write_all(&bytes[..n.min(bytes.len())])?;
return Err(io::Error::new(io::ErrorKind::StorageFull, "injected: no space left on device"));
}
RealFs.write_new(tmp, bytes)
}
fn rename(&self, from: &Path, to: &Path) -> io::Result<()> {
self.renames.set(self.renames.get() + 1);
if let Some(kind) = self.fail_rename {
return Err(io::Error::new(kind, "injected rename failure"));
}
if self.locked_renames.get() > 0 {
self.locked_renames.set(self.locked_renames.get() - 1);
return Err(io::Error::new(io::ErrorKind::PermissionDenied, "injected: sharing violation"));
}
RealFs.rename(from, to)
}
fn remove(&self, path: &Path) -> io::Result<()> {
RealFs.remove(path)
}
fn sync_dir(&self, dir: &Path) -> io::Result<()> {
RealFs.sync_dir(dir)
}
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()> {
RealFs.copy_permissions(from, to)
}
}
const NO_RETRY: RenameRetry = RenameRetry { attempts: 0, first_delay_ms: 0 };
#[test]
fn replaces_and_creates() {
let d = TempDir::new("ok");
let p = d.0.join("doc.psd");
atomic_write(&p, b"first").unwrap();
assert_eq!(std::fs::read(&p).unwrap(), b"first");
atomic_write(&p, b"second, longer").unwrap();
assert_eq!(std::fs::read(&p).unwrap(), b"second, longer");
assert_no_temp(&d.0);
}
#[test]
fn creates_missing_parent_folders() {
let d = TempDir::new("parent");
let p = d.0.join("a/b/out.png");
atomic_write(&p, b"x").unwrap();
assert_eq!(std::fs::read(&p).unwrap(), b"x");
}
#[test]
fn write_failure_halfway_keeps_original() {
let d = TempDir::new("write");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
let fs = Faulty { fail_write_after: Some(5), ..Default::default() };
let e = atomic_write_with(&fs, NO_RETRY, &p, &[7u8; 4096]).unwrap_err();
assert_eq!(e.kind(), io::ErrorKind::StorageFull);
assert!(e.to_string().contains("not changed"), "{e}");
assert_eq!(std::fs::read(&p).unwrap(), ORIGINAL);
assert_eq!(fs.renames.get(), 0, "never renames after a failed write");
assert_no_temp(&d.0);
}
#[test]
fn rename_failure_keeps_original() {
let d = TempDir::new("rename");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
let fs = Faulty { fail_rename: Some(io::ErrorKind::Other), ..Default::default() };
assert!(atomic_write_with(&fs, NO_RETRY, &p, b"new").is_err());
assert_eq!(std::fs::read(&p).unwrap(), ORIGINAL);
assert_no_temp(&d.0);
}
#[test]
fn cross_device_rename_errors_clearly_and_never_writes_in_place() {
let d = TempDir::new("exdev");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
let fs = Faulty { fail_rename: Some(io::ErrorKind::CrossesDevices), ..Default::default() };
let e = atomic_write_with(&fs, NO_RETRY, &p, b"new").unwrap_err();
assert!(e.to_string().contains("different volume"), "{e}");
assert_eq!(std::fs::read(&p).unwrap(), ORIGINAL);
assert_no_temp(&d.0);
}
#[test]
fn locked_target_rename_is_retried() {
let d = TempDir::new("locked");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
let fs = Faulty { locked_renames: Cell::new(2), ..Default::default() };
atomic_write_with(&fs, RenameRetry { attempts: 3, first_delay_ms: 1 }, &p, b"new").unwrap();
assert_eq!(std::fs::read(&p).unwrap(), b"new");
assert_eq!(fs.renames.get(), 3);
assert_no_temp(&d.0);
}
#[test]
fn lock_that_outlasts_the_retries_keeps_original() {
let d = TempDir::new("locked2");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
let fs = Faulty { locked_renames: Cell::new(10), ..Default::default() };
let e = atomic_write_with(&fs, RenameRetry { attempts: 2, first_delay_ms: 1 }, &p, b"new").unwrap_err();
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
assert!(e.to_string().contains("locked"), "{e}");
assert_eq!(fs.renames.get(), 3);
assert_eq!(std::fs::read(&p).unwrap(), ORIGINAL);
assert_no_temp(&d.0);
}
#[cfg(unix)]
#[test]
fn read_only_folder_errors_clearly_and_never_writes_in_place() {
use std::os::unix::fs::PermissionsExt;
let d = TempDir::new("ro");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
// The file itself stays writable: an in-place fallback would succeed and must not happen.
std::fs::set_permissions(&d.0, std::fs::Permissions::from_mode(0o555)).unwrap();
if std::fs::File::create(d.0.join("probe")).is_ok() {
return; // running as root: permissions aren't enforced
}
let e = atomic_write(&p, b"new").unwrap_err();
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
assert!(e.to_string().contains("not writable"), "{e}");
assert_eq!(std::fs::read(&p).unwrap(), ORIGINAL);
std::fs::set_permissions(&d.0, std::fs::Permissions::from_mode(0o755)).unwrap();
assert_no_temp(&d.0);
}
#[cfg(unix)]
#[test]
fn keeps_the_original_file_mode() {
use std::os::unix::fs::PermissionsExt;
let d = TempDir::new("mode");
let p = d.0.join("doc.psd");
std::fs::write(&p, ORIGINAL).unwrap();
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o600)).unwrap();
atomic_write(&p, b"new").unwrap();
assert_eq!(std::fs::metadata(&p).unwrap().permissions().mode() & 0o777, 0o600);
}
#[cfg(unix)]
#[test]
fn writes_through_a_symlink() {
let d = TempDir::new("link");
let real = d.0.join("real.psd");
let link = d.0.join("link.psd");
std::fs::write(&real, ORIGINAL).unwrap();
std::os::unix::fs::symlink(&real, &link).unwrap();
atomic_write(&link, b"new").unwrap();
assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink());
assert_eq!(std::fs::read(&real).unwrap(), b"new");
}
#[test]
fn directory_target_or_bare_root_is_an_error_not_a_panic() {
let d = TempDir::new("dir");
assert!(atomic_write(&d.0, b"x").is_err());
assert!(atomic_write(Path::new("/"), b"x").is_err());
assert!(atomic_write(Path::new(""), b"x").is_err());
}

2
crates/format/src/lib.rs
View File

@ -20,6 +20,7 @@
#![forbid(unsafe_code)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
pub mod atomic;
pub mod autosave;
mod convert;
pub mod manifest;
@ -32,6 +33,7 @@ use std::path::Path;
use photocraft_doc::Document;
use photocraft_raster::Rgba8Image;
pub use atomic::atomic_write;
pub use autosave::{Autosaver, RecoveryEntry, discard_recovery, list_recovery, recover};
pub use manifest::{FORMAT_VERSION, Manifest};
pub use store::{PcraftWriter, SaveStats};

8
crates/format/src/store.rs
View File

@ -448,12 +448,8 @@ fn list_objects(dir: &Path) -> Result<HashSet<String>> {
Ok(out)
}
/// Crash-safe replace of one file (see [`crate::atomic`]).
pub(crate) fn write_atomic(path: &Path, data: &[u8]) -> Result<()> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
}
let tmp = path.with_extension(format!("{}tmp", path.extension().map(|e| format!("{}.", e.to_string_lossy())).unwrap_or_default()));
std::fs::write(&tmp, data)?;
std::fs::rename(&tmp, path)?;
crate::atomic::atomic_write(path, data)?;
Ok(())
}

2
crates/ui-egui/examples/snapshot.rs
View File

@ -36,7 +36,7 @@ fn main() {
}
photocraft_io::export(doc, path, &opts).map(|r| (r.bytes, r.warnings)).map_err(|e| e.to_string())
})),
write: Some(Box::new(|path: &str, bytes: &[u8]| std::fs::write(path, bytes).map_err(|e| e.to_string()))),
write: Some(Box::new(|path: &str, bytes: &[u8]| photocraft_format::atomic_write(std::path::Path::new(path), bytes).map_err(|e| e.to_string()))),
..Default::default()
};
let open = arg(&args, "--open");