feat: atomic file saving

Cargo.toml

@@ -25,6 +25,7 @@ anyhow = "1.0"
 approx = "0.5.1"
 async-fs = "2.1"
 base64 = "0.22.1"
+blocking = "1.6"
 cairo-rs = { version = "0.21.1", features = ["v1_18", "png", "svg", "pdf"] }
 chrono = "0.4.41"
 clap = { version = "4.5", features = ["derive"] }
@@ -87,6 +88,7 @@ serde_json = "1.0"
 slotmap = { version = "1.0", features = ["serde"] }
 smol = "2.0"
 svg = "0.18.0"
+tempfile = "3.26"
 thiserror = "2.0.12"
 tracing = "0.1.41"
 tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }

crates/rnote-engine/Cargo.toml

@@ -51,6 +51,7 @@ serde = { workspace = true }
 serde_json = { workspace = true }
 slotmap = { workspace = true }
 svg = { workspace = true }
+tempfile = { workspace = true }
 thiserror = { workspace = true }
 tracing = { workspace = true }
 unicode-segmentation = { workspace = true }

crates/rnote-engine/src/utils.rs

@@ -1,10 +1,11 @@
 // Imports
 use crate::fileformats::xoppformat;
+use anyhow::Context;
 use geo::line_string;
 use hayro::hayro_syntax;
 use p2d::bounding_volume::Aabb;
 use rnote_compose::Color;
-use std::ops::Range;
+use std::{io::Write, ops::Range};
 
 pub const fn crate_version() -> &'static str {
     env!("CARGO_PKG_VERSION")
@@ -118,3 +119,52 @@ pub mod glib_bytes_base64 {
         rnote_compose::serialize::sliceu8_base64::deserialize(d).map(glib::Bytes::from_owned)
     }
 }
+
+/// Attempts to atomically save data to a file.
+/// Not asynchronous, wrap with `blocking::unblock()` or equivalent to avoid blocking.
+pub fn atomic_save_to_file<Q, B>(filepath: Q, bytes: B) -> anyhow::Result<()>
+where
+    Q: AsRef<std::path::Path>,
+    B: AsRef<[u8]>,
+{
+    let filepath = filepath.as_ref();
+    let bytes = bytes.as_ref();
+
+    let parent_directory = filepath
+        .parent()
+        .ok_or_else(|| anyhow::anyhow!("The filepath does not have a parent directory"))?;
+
+    // We first create the named temporary file, specifically in the parent
+    // directory of the target filepath, as `.persist()` will not work
+    // if the temporary file is in a different filesystem than the target.
+    let mut temp_file = tempfile::NamedTempFile::new_in(parent_directory)
+        .with_context(|| "Failed to create a temporary file")?;
+
+    // We then write all of our bytes to the temporary file before syncing its contents.
+    temp_file
+        .write_all(bytes)
+        .with_context(|| "Failed to write to the temporary file")?;
+    temp_file
+        .as_file()
+        .sync_all()
+        .with_context(|| "Failed to sync the contents and metadata of the temporary file")?;
+
+    // Finally, we persist the temporary file to the target filepath, if a file
+    // pre-exists at this location, it will be atomically replaced by our new file.
+    let _ = temp_file
+        .persist(filepath)
+        .with_context(|| "Failed to persist the temporary file to the target filepath")?;
+
+    #[cfg(unix)]
+    {
+        // On UNIX systems, we also sync the parent directory after the persist operation.
+        // Not required for Windows systems, not possible either (you can't open directories as files in the first place).
+        // Note that this might not even be enough, file management on UNIX seems to be a bit of a nightmare.
+        std::fs::File::open(parent_directory)
+            .with_context(|| "Failed to open the parent directory")?
+            .sync_all()
+            .with_context(|| "Failed to sync the contents and metadata of the parent directory")?;
+    }
+
+    Ok(())
+}

crates/rnote-ui/Cargo.toml

@@ -18,6 +18,7 @@ anyhow = { workspace = true }
 approx = { workspace = true }
 async-fs = { workspace = true }
 base64 = { workspace = true }
+blocking = { workspace = true }
 cairo-rs = { workspace = true }
 chrono = { workspace = true }
 fs_extra = { workspace = true }

crates/rnote-ui/src/canvas/imexport.rs

@@ -1,8 +1,6 @@
 // Imports
 use super::RnCanvas;
 use crate::RnAppWindow;
-use anyhow::Context;
-use futures::AsyncWriteExt;
 use futures::channel::oneshot;
 use gtk4::{gio, prelude::*};
 use rnote_compose::ext::Vector2Ext;
@@ -211,8 +209,10 @@ impl RnCanvas {
 
     /// Saves the document to the given file.
     ///
-    /// Returns Ok(true) if saved successfully, Ok(false) when a save is already in progress and no file operatiosn were
-    /// executed, Err(e) when saving failed in any way.
+    /// Returns:
+    /// - `Ok(true)` if saving was sucessful
+    /// - `Ok(false)` if a save was already in progress (and thus this function didn't do anything)
+    /// - `Err(e)` when saving failed in any way
     #[tracing::instrument(skip_all, fields(path = format!("{:?}", file.path())))]
     pub(crate) async fn save_document_to_file(&self, file: &gio::File) -> anyhow::Result<bool> {
         // skip saving when it is already in progress
@@ -223,7 +223,7 @@ impl RnCanvas {
         self.set_save_in_progress(true);
         debug!("Saving file is now in progress");
 
-        let file_path = file.path().ok_or_else(|| {
+        let filepath = file.path().ok_or_else(|| {
             self.set_save_in_progress(false);
             anyhow::anyhow!("Could not get a path for file: `{file:?}`.")
         })?;
@@ -234,40 +234,13 @@ impl RnCanvas {
         let rnote_bytes_receiver = self
             .engine_ref()
             .save_as_rnote_bytes(basename.to_string_lossy().to_string());
-        let mut skip_set_output_file = false;
-        if let Some(output_file_path) = self.output_file().and_then(|f| f.path())
-            && crate::utils::paths_abs_eq(output_file_path, &file_path).unwrap_or(false)
-        {
-            skip_set_output_file = true;
-        }
+
         self.dismiss_output_file_modified_toast();
 
-        let file_write_operation = async move {
+        let file_write_operation = async {
             let bytes = rnote_bytes_receiver.await??;
             self.set_output_file_expect_write(true);
-            let mut write_file = async_fs::OpenOptions::new()
-                .create(true)
-                .truncate(true)
-                .write(true)
-                .open(&file_path)
-                .await
-                .context(format!(
-                    "Failed to create/open/truncate file for path '{}'",
-                    file_path.display()
-                ))?;
-            if !skip_set_output_file {
-                // this installs the file watcher.
-                self.set_output_file(Some(file.to_owned()));
-            }
-            write_file.write_all(&bytes).await.context(format!(
-                "Failed to write bytes to file with path '{}'",
-                file_path.display()
-            ))?;
-            write_file.sync_all().await.context(format!(
-                "Failed to sync file after writing with path '{}'",
-                file_path.display()
-            ))?;
-            Ok(())
+            crate::utils::atomic_save_to_file_future(&filepath, bytes).await
         };
 
         if let Err(e) = file_write_operation.await {
@@ -278,6 +251,8 @@ impl RnCanvas {
             return Err(e);
         }
 
+        // Required, as atomic file saving moves a new file into the old one
+        self.set_output_file(Some(gio::File::for_path(&filepath)));
         debug!("Saving file has finished successfully");
         self.set_unsaved_changes(false);
         self.set_save_in_progress(false);

crates/rnote-ui/src/utils.rs

@@ -15,6 +15,16 @@ pub(crate) const FILE_DUP_SUFFIX_DELIM: &str = " - ";
 /// The suffix delimiter when duplicating/renaming already existing files for usage in a regular expression
 pub(crate) const FILE_DUP_SUFFIX_DELIM_REGEX: &str = r"\s-\s";
 
+/// An asynchronous adaptation of the [`rnote_engine::utils::atomic_save_to_file`] function.
+pub(crate) async fn atomic_save_to_file_future<Q>(filepath: Q, bytes: Vec<u8>) -> anyhow::Result<()>
+where
+    Q: AsRef<std::path::Path>,
+{
+    let filepath = filepath.as_ref().to_path_buf();
+
+    blocking::unblock(move || rnote_engine::utils::atomic_save_to_file(filepath, bytes)).await
+}
+
 /// Create a new file or replace if it already exists, asynchronously.
 pub(crate) async fn create_replace_file_future(
     bytes: Vec<u8>,