From 859121bcbc2106058b4ce9f2cd4f079532b35a24 Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Fri, 11 Jun 2021 07:52:00 -0700 Subject: [PATCH] Add atomic file saving Adapted from the atomicwrites crate, but with directory fsync disabled for performance. --- Cargo.lock | 28 ++++- Cargo.toml | 2 + src/apps/user.rs | 53 ++++---- src/common/atomic_save.rs | 249 ++++++++++++++++++++++++++++++++++++++ src/common/mod.rs | 1 + 5 files changed, 309 insertions(+), 24 deletions(-) create mode 100644 src/common/atomic_save.rs diff --git a/Cargo.lock b/Cargo.lock index 22451c6..71e45be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -383,11 +383,13 @@ dependencies = [ "json", "mime", "mime-db", + "nix", "once_cell", "pest", "pest_derive", "serde", "shlex", + "tempfile", "thiserror", "url", "xdg", @@ -567,9 +569,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.94" +version = "0.2.96" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18794a8ad5b29321f790b55d93dfba91e125cb1a9edbd4f8e3150acc771c1a5e" +checksum = "5600b4e6efc5421841a2138a6b082e07fe12f9aaa12783d50e5d13325b26b4fc" [[package]] name = "log" @@ -598,6 +600,15 @@ version = "2.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" +[[package]] +name = "memoffset" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59accc507f1338036a0477ef61afdae33cde60840f4dfe481319ce3ad116ddf9" +dependencies = [ + "autocfg", +] + [[package]] name = "mime" version = "0.3.16" @@ -655,6 +666,19 @@ dependencies = [ "tempfile", ] +[[package]] +name = "nix" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c3728fec49d363a50a8828a190b379a446cc5cf085c06259bbbeb34447e4ec7" +dependencies = [ + "bitflags", + "cc", + "cfg-if 1.0.0", + "libc", + "memoffset", +] + [[package]] name = "nom" version = "5.1.2" diff --git a/Cargo.toml b/Cargo.toml index 1fc6bca..987c91d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,8 @@ xdg-mime = "0.3.3" freedesktop_entry_parser = "1.1.1" once_cell = "1.7.2" aho-corasick = "0.7.15" +tempfile = "3.2.0" +nix = "0.21.0" [profile.release] opt-level=3 diff --git a/src/apps/user.rs b/src/apps/user.rs index a778673..9f6b316 100644 --- a/src/apps/user.rs +++ b/src/apps/user.rs @@ -1,3 +1,6 @@ +use crate::common::atomic_save::{ + AtomicFile, AtomicSaveError, Durability, OverwriteBehavior, +}; use crate::{apps::SystemApps, common::Handler, Error, Result, CONFIG}; use mime::Mime; use once_cell::sync::Lazy; @@ -179,31 +182,37 @@ impl MimeApps { use itertools::Itertools; use std::io::{prelude::*, BufWriter}; - let f = std::fs::OpenOptions::new() - .read(true) - .create(true) - .write(true) - .truncate(true) - .open(Self::path()?)?; - let mut writer = BufWriter::new(f); + let af = AtomicFile::new( + &Self::path()?, + OverwriteBehavior::AllowOverwrite, + Durability::DontSyncDir, + ); + af.write(|f| -> Result<()> { + let mut writer = BufWriter::new(f); - writer.write_all(b"[Added Associations]\n")?; - for (k, v) in self.added_associations.iter().sorted() { - writer.write_all(k.essence_str().as_ref())?; - writer.write_all(b"=")?; - writer.write_all(v.iter().join(";").as_ref())?; - writer.write_all(b";\n")?; - } + writer.write_all(b"[Added Associations]\n")?; + for (k, v) in self.added_associations.iter().sorted() { + writer.write_all(k.essence_str().as_ref())?; + writer.write_all(b"=")?; + writer.write_all(v.iter().join(";").as_ref())?; + writer.write_all(b";\n")?; + } - writer.write_all(b"\n[Default Applications]\n")?; - for (k, v) in self.default_apps.iter().sorted() { - writer.write_all(k.essence_str().as_ref())?; - writer.write_all(b"=")?; - writer.write_all(v.iter().join(";").as_ref())?; - writer.write_all(b";\n")?; - } + writer.write_all(b"\n[Default Applications]\n")?; + for (k, v) in self.default_apps.iter().sorted() { + writer.write_all(k.essence_str().as_ref())?; + writer.write_all(b"=")?; + writer.write_all(v.iter().join(";").as_ref())?; + writer.write_all(b";\n")?; + } - writer.flush()?; + writer.flush()?; + Ok(()) + }) + .map_err(|e| match e { + AtomicSaveError::Internal(e) => Error::Io(e), + AtomicSaveError::User(e) => e, + })?; Ok(()) } pub fn print(&self, detailed: bool) -> Result<()> { diff --git a/src/common/atomic_save.rs b/src/common/atomic_save.rs new file mode 100644 index 0000000..d28ab34 --- /dev/null +++ b/src/common/atomic_save.rs @@ -0,0 +1,249 @@ +//! Adapted from https://github.com/untitaker/rust-atomicwrites/blob/master/src/lib.rs. + +use std::error::Error as ErrorTrait; +use std::fmt; +use std::fs; +use std::io; +use std::path; + +pub use OverwriteBehavior::{AllowOverwrite, DisallowOverwrite}; + +/// Whether to allow overwriting if the target file exists. +#[derive(Clone, Copy)] +pub enum OverwriteBehavior { + /// Overwrite files silently. + AllowOverwrite, + + /// Don't overwrite files. `AtomicFile.write` will raise errors for such conditions only after + /// you've already written your data. + DisallowOverwrite, +} + +/// Whether to ensure durability after a system crash (guaranteed to contain the new data). +/// Regardless of the option you pick, the atomic write will be consistent after a crash +/// (will never contain partially-written data). +#[derive(Clone, Copy)] +pub enum Durability { + /// Faster, ensures either old or new file contents (but not half-written data) + /// will be present after system crash. + DontSyncDir, + /// Slower, ensures new file contents will be present after system crash. + /// Not possible on Windows. + SyncDir, +} + +/// Represents an error raised by `AtomicFile.write`. +#[derive(Debug)] +pub enum AtomicSaveError { + /// The error originated in the library itself, while it was either creating a temporary file + /// or moving the file into place. + Internal(io::Error), + /// The error originated in the user-supplied callback. + User(E), +} + +/// If your callback returns a `std::io::Error`, you can unwrap this type to `std::io::Error`. +impl From> for io::Error { + fn from(e: AtomicSaveError) -> Self { + match e { + AtomicSaveError::Internal(x) => x, + AtomicSaveError::User(x) => x, + } + } +} + +impl fmt::Display for AtomicSaveError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + AtomicSaveError::Internal(ref e) => e.fmt(f), + AtomicSaveError::User(ref e) => e.fmt(f), + } + } +} + +impl ErrorTrait for AtomicSaveError { + fn cause(&self) -> Option<&dyn ErrorTrait> { + match *self { + AtomicSaveError::Internal(ref e) => Some(e), + AtomicSaveError::User(ref e) => Some(e), + } + } +} + +fn safe_parent(p: &path::Path) -> Option<&path::Path> { + match p.parent() { + None => None, + Some(x) if x.as_os_str().len() == 0 => Some(&path::Path::new(".")), + x => x, + } +} + +/// Create a file and write to it atomically, in a callback. +pub struct AtomicFile { + /// Path to the final file that is atomically written. + path: path::PathBuf, + overwrite: OverwriteBehavior, + durability: Durability, + /// Directory to which to write the temporary subdirectories. + tmpdir: path::PathBuf, +} + +impl AtomicFile { + /// Helper for writing to the file at `path` atomically, in write-only mode. + /// + /// If `OverwriteBehaviour::DisallowOverwrite` is given, + /// an `Error::Internal` containing an `std::io::ErrorKind::AlreadyExists` + /// will be returned from `self.write(...)` if the file exists. + /// + /// The temporary file is written to a temporary subdirectory in `.`, to ensure + /// it’s on the same filesystem (so that the move is atomic). + pub fn new( + p: &path::Path, + overwrite: OverwriteBehavior, + durability: Durability, + ) -> Self { + AtomicFile::new_with_tmpdir( + p, + overwrite, + durability, + safe_parent(p).unwrap_or(path::Path::new(".")), + ) + } + + /// Like `AtomicFile::new`, but the temporary file is written to a temporary subdirectory in `tmpdir`. + /// + /// TODO: does `tmpdir` have to exist? + pub fn new_with_tmpdir( + path: &path::Path, + overwrite: OverwriteBehavior, + durability: Durability, + tmpdir: &path::Path, + ) -> Self { + AtomicFile { + path: path.to_path_buf(), + overwrite, + durability, + tmpdir: tmpdir.to_path_buf(), + } + } + + /// Move the file to `self.path()`. Not exposed! + fn commit(self, tmppath: &path::Path) -> io::Result<()> { + match self.overwrite { + AllowOverwrite => { + replace_atomic(tmppath, self.path(), self.durability) + } + DisallowOverwrite => { + move_atomic(tmppath, self.path(), self.durability) + } + } + } + + /// Get the target filepath. + pub fn path(&self) -> &path::Path { + &self.path + } + + /// Open a temporary file, call `f` on it (which is supposed to write to it), then move the + /// file atomically to `self.path`. + /// + /// The temporary file is written to a randomized temporary subdirectory with prefix `.atomicwrite`. + pub fn write(self, f: F) -> Result> + where + F: FnOnce(&mut fs::File) -> Result, + { + let tmpdir = tempfile::Builder::new() + .prefix(".atomicwrite") + .tempdir_in(&self.tmpdir) + .map_err(AtomicSaveError::Internal)?; + + let tmppath = tmpdir.path().join("tmpfile.tmp"); + let rv = { + let mut tmpfile = fs::File::create(&tmppath) + .map_err(AtomicSaveError::Internal)?; + let r = f(&mut tmpfile).map_err(AtomicSaveError::User)?; + tmpfile.sync_all().map_err(AtomicSaveError::Internal)?; + r + }; + self.commit(&tmppath).map_err(AtomicSaveError::Internal)?; + Ok(rv) + } +} + +mod imp { + use super::{safe_parent, Durability}; + + use std::os::unix::io::AsRawFd; + use std::{fs, io, path}; + + fn fsync(f: T) -> io::Result<()> { + match nix::unistd::fsync(f.as_raw_fd()) { + Ok(()) => Ok(()), + Err(nix::Error::Sys(errno)) => Err(errno.into()), + Err(nix::Error::InvalidPath) => { + Err(io::Error::new(io::ErrorKind::Other, "invalid path")) + } + Err(nix::Error::InvalidUtf8) => { + Err(io::Error::new(io::ErrorKind::Other, "invalid utf-8")) + } + Err(nix::Error::UnsupportedOperation) => Err(io::Error::new( + io::ErrorKind::Other, + "unsupported operation", + )), + } + } + + fn fsync_dir(x: &path::Path) -> io::Result<()> { + let f = fs::File::open(x)?; + fsync(f) + } + + /// Move `src` to `dst`. If `dst` exists, it will be silently overwritten. + /// + /// Both paths must reside on the same filesystem for the operation to be atomic. + pub fn replace_atomic( + src: &path::Path, + dst: &path::Path, + durability: Durability, + ) -> io::Result<()> { + fs::rename(src, dst)?; + + match durability { + Durability::SyncDir => { + let dst_directory = safe_parent(dst).unwrap(); + fsync_dir(dst_directory)?; + } + Durability::DontSyncDir => {} + } + + Ok(()) + } + + /// Move `src` to `dst`. An error will be returned if `dst` exists. + /// + /// Both paths must reside on the same filesystem for the operation to be atomic. + pub fn move_atomic( + src: &path::Path, + dst: &path::Path, + durability: Durability, + ) -> io::Result<()> { + fs::hard_link(src, dst)?; + fs::remove_file(src)?; + + match durability { + Durability::SyncDir => { + let src_directory = safe_parent(src).unwrap(); + let dst_directory = safe_parent(dst).unwrap(); + fsync_dir(dst_directory)?; + if src_directory != dst_directory { + fsync_dir(src_directory)?; + } + } + Durability::DontSyncDir => {} + } + + Ok(()) + } +} + +use imp::{move_atomic, replace_atomic}; diff --git a/src/common/mod.rs b/src/common/mod.rs index 3f065e2..70727cb 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -1,3 +1,4 @@ +pub mod atomic_save; mod db; mod desktop_entry; mod handler;