mirror of
https://github.com/storytold/figcraft.git
synced 2026-10-27 11:56:50 -06:00
Merge #284: fix(format): exclusive unique staging files for atomic save/export (@notdezzi)
This commit is contained in:
bflatastic
@@ -437,15 +437,12 @@ pub(crate) fn read_capped(path: &str, max: u64) -> Result<Vec<u8>> {
|
||||
read(path)
|
||||
}
|
||||
|
||||
/// Write `bytes` to `path` through a uniquely named, exclusively created staging file and one
|
||||
/// atomic rename (see [`figcraft_format::write_file_atomic`]): a failed or interrupted write never
|
||||
/// truncates `path`, and no file or symlink at a staging name is ever written through.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub(crate) fn write(path: &str, bytes: &[u8]) -> Result<()> {
|
||||
let tmp = format!("{path}.tmp-figcraft");
|
||||
std::fs::write(&tmp, bytes).map_err(|e| EngineError::Other(format!("{tmp}: {e}")))?;
|
||||
std::fs::rename(&tmp, path).map_err(|e| {
|
||||
// Leave nothing behind when the file can't take its place.
|
||||
let _ = std::fs::remove_file(&tmp);
|
||||
EngineError::Other(format!("{path}: {e}"))
|
||||
})
|
||||
figcraft_format::write_file_atomic(std::path::Path::new(path), bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))
|
||||
}
|
||||
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
@@ -1327,3 +1324,126 @@ mod rest_import_tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod staging_write_tests {
|
||||
use super::*;
|
||||
|
||||
fn dir(name: &str) -> std::path::PathBuf {
|
||||
let d = std::env::temp_dir().join(format!("figcraft-engine-staging-{name}-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&d).unwrap();
|
||||
d
|
||||
}
|
||||
|
||||
/// A symlink `from` → `to`; false when the platform won't make one (Windows needs privileges).
|
||||
#[cfg(unix)]
|
||||
fn symlink(from: &std::path::Path, to: &std::path::Path) -> bool {
|
||||
std::os::unix::fs::symlink(to, from).is_ok()
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn symlink(from: &std::path::Path, to: &std::path::Path) -> bool {
|
||||
std::os::windows::fs::symlink_file(to, from).is_ok()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_existing_staging_file_is_never_written_through() {
|
||||
let d = dir("existing");
|
||||
let path = d.join("shot.svg");
|
||||
// The old deterministic staging name, held by an unrelated file: the write must pick its
|
||||
// own staging name instead of truncating this one.
|
||||
let old = d.join("shot.svg.tmp-figcraft");
|
||||
std::fs::write(&old, b"not yours").unwrap();
|
||||
write(&path.to_string_lossy(), b"<svg/>").unwrap();
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"<svg/>");
|
||||
assert_eq!(std::fs::read(&old).unwrap(), b"not yours", "an unrelated file at a staging name is untouched");
|
||||
// Just the file and the untouched staging file: no new staging leftovers.
|
||||
assert_eq!(std::fs::read_dir(&d).unwrap().count(), 2);
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_staging_symlink_is_never_followed() {
|
||||
let d = dir("symlink");
|
||||
let path = d.join("shot.svg");
|
||||
let victim = d.join("victim.txt");
|
||||
std::fs::write(&victim, b"innocent").unwrap();
|
||||
let old = d.join("shot.svg.tmp-figcraft");
|
||||
if !symlink(&old, &victim) {
|
||||
return; // Making a symlink needs privileges on some Windows setups.
|
||||
}
|
||||
write(&path.to_string_lossy(), b"<svg/>").unwrap();
|
||||
assert_eq!(std::fs::read(&victim).unwrap(), b"innocent", "the symlink was never written through");
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"<svg/>");
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_writers_of_one_file_never_interleave() {
|
||||
let d = dir("concurrent");
|
||||
let path = d.join("shot.svg");
|
||||
for round in 0..5u8 {
|
||||
let mut handles = Vec::new();
|
||||
for w in 0u8..4 {
|
||||
let (path, bytes) = (path.clone(), vec![b'0' + w; 8192]);
|
||||
handles.push(std::thread::spawn(move || write(&path.to_string_lossy(), &bytes)));
|
||||
}
|
||||
let mut wins = 0;
|
||||
for h in handles {
|
||||
// On Windows a rename over a file another writer just replaced can lose the race.
|
||||
if h.join().unwrap().is_ok() {
|
||||
wins += 1;
|
||||
}
|
||||
}
|
||||
assert!(wins > 0, "round {round}: every writer lost");
|
||||
// Whatever landed is one writer's complete payload, never a mix.
|
||||
let got = std::fs::read(&path).unwrap();
|
||||
assert_eq!(got.len(), 8192, "round {round}: a complete payload landed");
|
||||
assert!(got.windows(2).all(|w| w[0] == w[1]), "round {round}: one writer's payload, not a mix");
|
||||
}
|
||||
// No staging file was left stuck (a failed writer removes its own).
|
||||
let left = std::fs::read_dir(&d).unwrap().count();
|
||||
assert!(left <= if cfg!(windows) { 5 } else { 1 }, "{left} files left in the staging directory");
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_write_leaves_the_destination_alone_and_cleans_up() {
|
||||
let d = dir("failure");
|
||||
// The destination is a directory: the rename fails, our staging file is removed and the
|
||||
// directory is untouched.
|
||||
let taken = d.join("over-a-directory");
|
||||
std::fs::create_dir_all(&taken).unwrap();
|
||||
assert!(write(&taken.to_string_lossy(), b"payload").is_err());
|
||||
assert!(taken.is_dir());
|
||||
assert_eq!(std::fs::read_dir(&d).unwrap().count(), 1, "no staging file left behind");
|
||||
// A missing parent directory fails without creating anything anywhere.
|
||||
assert!(write(&d.join("no").join("such.svg").to_string_lossy(), b"payload").is_err());
|
||||
assert!(!d.join("no").exists());
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
// A read-only directory refuses the staging file, and the old destination stays.
|
||||
let ro = d.join("read-only");
|
||||
std::fs::create_dir_all(&ro).unwrap();
|
||||
let dest = ro.join("keep.svg");
|
||||
std::fs::write(&dest, b"keep").unwrap();
|
||||
let mut perms = std::fs::metadata(&ro).unwrap().permissions();
|
||||
perms.set_mode(0o500);
|
||||
std::fs::set_permissions(&ro, perms).unwrap();
|
||||
// Running as root would defeat the read-only directory: skip instead of failing.
|
||||
let probe = ro.join(".probe");
|
||||
if std::fs::write(&probe, b"").is_ok() {
|
||||
let _ = std::fs::remove_file(&probe);
|
||||
} else {
|
||||
assert!(write(&dest.to_string_lossy(), b"payload").is_err());
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), b"keep");
|
||||
assert_eq!(std::fs::read_dir(&ro).unwrap().count(), 1, "no staging file left behind");
|
||||
}
|
||||
let mut perms = std::fs::metadata(&ro).unwrap().permissions();
|
||||
perms.set_mode(0o700);
|
||||
std::fs::set_permissions(&ro, perms).unwrap();
|
||||
}
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -654,14 +654,53 @@ fn read_branches<R: Read + Seek>(z: &mut zip::ZipArchive<R>, images: &BTreeMap<S
|
||||
out
|
||||
}
|
||||
|
||||
/// Save atomically: write a temp file next to `path`, then rename over it.
|
||||
/// Save atomically: write a staging file next to `path`, then rename over it.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub fn save_file(doc: &Document, path: &std::path::Path, thumbnail_png: Option<&[u8]>) -> Result<(), FormatError> {
|
||||
let bytes = save_bytes(doc, thumbnail_png)?;
|
||||
let tmp = path.with_extension("figcraft.tmp");
|
||||
std::fs::write(&tmp, &bytes).map_err(|e| FormatError::Io(format!("{}: {e}", tmp.display())))?;
|
||||
std::fs::rename(&tmp, path).map_err(|e| FormatError::Io(format!("{}: {e}", path.display())))?;
|
||||
Ok(())
|
||||
write_file_atomic(path, &bytes).map_err(|e| FormatError::Io(format!("{}: {e}", path.display())))
|
||||
}
|
||||
|
||||
/// Write `bytes` to `path` through a staging file and one atomic rename, so a reader of `path`
|
||||
/// sees either the previous file or the complete new one, never a partial write. The staging file
|
||||
/// is a uniquely named sibling (`<path>.tmp-figcraft-<process id>-<counter>`) created exclusively
|
||||
/// (`create_new`) and written through its own handle: a file or symlink that already exists at a
|
||||
/// staging name is never truncated or followed, and separate writers (also of separate processes)
|
||||
/// never share a staging file. On failure only the staging file this call created is removed, and
|
||||
/// `path` is replaced only on success. The rename replaces an existing destination on every
|
||||
/// platform (`std::fs::rename` uses `MoveFileEx` with `MOVEFILE_REPLACE_EXISTING` on Windows).
|
||||
/// (No `sync_all` yet: nothing around here promises durability across a power cut.)
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub fn write_file_atomic(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> {
|
||||
use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
|
||||
|
||||
static STAGING: AtomicU64 = AtomicU64::new(0);
|
||||
// Three tries at a fresh name: the counter is unique within this process, so only a leftover
|
||||
// of a dead process that now shares our process id can collide, and the next counter won't.
|
||||
for _ in 0..3 {
|
||||
let mut name = path.as_os_str().to_os_string();
|
||||
name.push(format!(".tmp-figcraft-{}-{}", std::process::id(), STAGING.fetch_add(1, Relaxed)));
|
||||
let tmp = std::path::PathBuf::from(name);
|
||||
match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
|
||||
Ok(mut file) => {
|
||||
if let Err(e) = file.write_all(bytes) {
|
||||
let _ = std::fs::remove_file(&tmp);
|
||||
return Err(e);
|
||||
}
|
||||
return match std::fs::rename(&tmp, path) {
|
||||
Ok(()) => Ok(()),
|
||||
// Leave nothing behind when the file can't take its place.
|
||||
Err(e) => {
|
||||
let _ = std::fs::remove_file(&tmp);
|
||||
Err(e)
|
||||
}
|
||||
};
|
||||
}
|
||||
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
}
|
||||
Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "no free staging name next to the file"))
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
@@ -677,6 +716,8 @@ mod image_tests;
|
||||
#[cfg(test)]
|
||||
mod large_tests;
|
||||
#[cfg(test)]
|
||||
mod staging_tests;
|
||||
#[cfg(test)]
|
||||
mod thumbnail_tests;
|
||||
#[cfg(test)]
|
||||
mod version_tests;
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
//! The staging write behind `save_file`: a pre-existing file or symlink at a staging name is never
|
||||
//! truncated or followed, concurrent savers of one file never corrupt it, and a failed write
|
||||
//! cleans up after itself and leaves the destination alone.
|
||||
|
||||
use figcraft_doc::Document;
|
||||
|
||||
use crate::{save_file, write_file_atomic};
|
||||
|
||||
fn dir(name: &str) -> std::path::PathBuf {
|
||||
let d = std::env::temp_dir().join(format!("figcraft-staging-tests-{name}-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&d).unwrap();
|
||||
d
|
||||
}
|
||||
|
||||
/// A symlink `from` → `to`; false when the platform won't make one (Windows needs privileges).
|
||||
#[cfg(unix)]
|
||||
fn symlink(from: &std::path::Path, to: &std::path::Path) -> bool {
|
||||
std::os::unix::fs::symlink(to, from).is_ok()
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn symlink(from: &std::path::Path, to: &std::path::Path) -> bool {
|
||||
std::os::windows::fs::symlink_file(to, from).is_ok()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_existing_staging_file_is_never_written_through() {
|
||||
let d = dir("existing");
|
||||
let path = d.join("a.figcraft");
|
||||
// The old deterministic staging name, held by an unrelated file: saving must pick its own
|
||||
// staging name instead of truncating this one.
|
||||
let old = d.join("a.figcraft.tmp");
|
||||
std::fs::write(&old, b"not yours").unwrap();
|
||||
save_file(&Document::new("Mine"), &path, None).unwrap();
|
||||
assert_eq!(std::fs::read(&old).unwrap(), b"not yours", "an unrelated file at a staging name is untouched");
|
||||
assert_eq!(crate::load_file(&path).unwrap().0.name, "Mine");
|
||||
// Just the file and the untouched staging file: no new staging leftovers.
|
||||
assert_eq!(std::fs::read_dir(&d).unwrap().count(), 2);
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_staging_symlink_is_never_followed() {
|
||||
let d = dir("symlink");
|
||||
let path = d.join("a.figcraft");
|
||||
let victim = d.join("victim.txt");
|
||||
std::fs::write(&victim, b"innocent").unwrap();
|
||||
let old = d.join("a.figcraft.tmp");
|
||||
if !symlink(&old, &victim) {
|
||||
return; // Making a symlink needs privileges on some Windows setups.
|
||||
}
|
||||
save_file(&Document::new("Mine"), &path, None).unwrap();
|
||||
assert_eq!(std::fs::read(&victim).unwrap(), b"innocent", "the symlink was never written through");
|
||||
assert_eq!(crate::load_file(&path).unwrap().0.name, "Mine");
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_savers_of_one_file_never_interleave() {
|
||||
let d = dir("concurrent");
|
||||
let path = d.join("a.figcraft");
|
||||
let names = ["W0", "W1", "W2", "W3"];
|
||||
for round in 0..5 {
|
||||
let mut handles = Vec::new();
|
||||
for name in names {
|
||||
let (path, doc) = (path.clone(), Document::new(name));
|
||||
handles.push(std::thread::spawn(move || save_file(&doc, &path, None)));
|
||||
}
|
||||
for h in handles {
|
||||
// On Windows a rename over a file another writer just replaced can lose the race.
|
||||
if cfg!(windows) {
|
||||
let _ = h.join().unwrap();
|
||||
} else {
|
||||
h.join().unwrap().unwrap();
|
||||
}
|
||||
}
|
||||
// Whatever landed is one writer's complete file, never a mix: a torn archive wouldn't load.
|
||||
let (doc, _) = crate::load_file(&path).unwrap();
|
||||
assert!(names.contains(&doc.name.as_str()), "round {round}: {:?}", doc.name);
|
||||
}
|
||||
// No staging file was left stuck.
|
||||
assert_eq!(std::fs::read_dir(&d).unwrap().count(), 1);
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_write_leaves_the_destination_alone_and_cleans_up() {
|
||||
let d = dir("failure");
|
||||
// The destination is a directory: the rename fails, our staging file is removed, the
|
||||
// directory is untouched.
|
||||
let taken = d.join("folder.figcraft");
|
||||
std::fs::create_dir_all(&taken).unwrap();
|
||||
assert!(save_file(&Document::new("Mine"), &taken, None).is_err());
|
||||
assert!(write_file_atomic(&taken, b"payload").is_err());
|
||||
assert!(taken.is_dir());
|
||||
assert_eq!(std::fs::read_dir(&d).unwrap().count(), 1, "no staging file left behind");
|
||||
// A missing parent directory fails without creating anything anywhere.
|
||||
assert!(write_file_atomic(&d.join("no").join("such.figcraft"), b"payload").is_err());
|
||||
assert!(!d.join("no").exists());
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
// A read-only directory refuses the staging file, and the old destination stays.
|
||||
let ro = d.join("read-only");
|
||||
std::fs::create_dir_all(&ro).unwrap();
|
||||
let dest = ro.join("keep.figcraft");
|
||||
std::fs::write(&dest, b"keep").unwrap();
|
||||
let mut perms = std::fs::metadata(&ro).unwrap().permissions();
|
||||
perms.set_mode(0o500);
|
||||
std::fs::set_permissions(&ro, perms).unwrap();
|
||||
// Running as root would defeat the read-only directory: skip instead of failing.
|
||||
let probe = ro.join(".probe");
|
||||
if std::fs::write(&probe, b"").is_ok() {
|
||||
let _ = std::fs::remove_file(&probe);
|
||||
} else {
|
||||
assert!(write_file_atomic(&dest, b"payload").is_err());
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), b"keep");
|
||||
assert_eq!(std::fs::read_dir(&ro).unwrap().count(), 1, "no staging file left behind");
|
||||
}
|
||||
let mut perms = std::fs::metadata(&ro).unwrap().permissions();
|
||||
perms.set_mode(0o700);
|
||||
std::fs::set_permissions(&ro, perms).unwrap();
|
||||
}
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
Reference in New Issue
Block a user