From bbc1551ac791b6adc6684d2c8295ef97062db8c1 Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Fri, 14 Nov 2025 14:49:08 +0100 Subject: [PATCH 1/8] Atomic updates for the config and state files and locking of the config file. --- Cargo.toml | 2 + src/commands/keyset/cmd.rs | 196 ++++++++++++++++++++++++++++--------- 2 files changed, 152 insertions(+), 46 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e87279d..042a7a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,6 +66,8 @@ tracing = "0.1.41" tracing-subscriber = "0.3.19" url = "2.5.4" futures = "0.3.31" +fs2 = "0.4.3" +nix = { version = "0.30.1", features = ["fs"] } [dev-dependencies] const_format = " 0.2.33" diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 6c7b44e..0a0f5d8 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -55,17 +55,20 @@ use domain::resolv::StubResolver; #[cfg(feature = "kmip")] use domain::utils::base32::encode_string_hex; use domain::zonefile::inplace::{Entry, Zonefile}; +use fs2::FileExt; use futures::future::join_all; use jiff::{Span, SpanRelativeTo}; +use nix::sys::stat::fstat; use serde::{Deserialize, Serialize}; use std::cmp::max; use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::From; use std::ffi::OsStr; use std::fmt::{Debug, Display, Formatter}; -use std::fs::{create_dir_all, remove_file, File}; +use std::fs::{create_dir_all, metadata, remove_file, rename, File}; use std::io::{self, Write}; use std::net::{IpAddr, SocketAddr}; +use std::os::fd::AsFd; use std::path::{absolute, Path, PathBuf}; use std::process::Command; use std::sync::Mutex; @@ -84,6 +87,9 @@ use super::kmip::{format_key_label, kmip_command, KmipCommands, KmipState}; /// with the key tags of existing keys. const MAX_KEY_TAG_TRIES: u8 = 10; +/// Number of times to try locking a file. +const MAX_FILE_LOCK_TRIES: u8 = 10; + /// Wait this amount before retrying for network errors, DNS errors, etc. const DEFAULT_WAIT: Duration = Duration::from_secs(10 * 60); @@ -621,6 +627,9 @@ struct WorkSpace { /// Whether the command to update DS records has to be executed. run_update_ds_command: bool, + /// Store the locked config file to avoid accidental unlocking. + _locked_config_file: Option, + #[cfg(feature = "kmip")] /// The current set of KMIP server pools. pools: HashMap, @@ -702,32 +711,26 @@ impl Keyset { ) })?; - let json = serde_json::to_string_pretty(&kss).expect("should not fail"); - let mut file = File::create(&state_file) - .map_err(|e| format!("unable to create file {}: {e}", state_file.display()))?; - write!(file, "{json}") - .map_err(|e| format!("unable to write to file {}: {e}", state_file.display()))?; + let ws = WorkSpace { + config: ksc, + state: kss, + config_changed: false, + state_changed: false, + run_update_ds_command: false, + _locked_config_file: None, + #[cfg(feature = "kmip")] + pools: HashMap::new(), + }; + + ws.write_state()?; + ws.write_config(&self.keyset_conf)?; - let json = serde_json::to_string_pretty(&ksc).expect("should not fail"); - let mut file = File::create(&self.keyset_conf).map_err(|e| { - format!("unable to create file {}: {e}", self.keyset_conf.display()) - })?; - write!(file, "{json}").map_err(|e| { - format!( - "unable to write to file {}: {e}", - self.keyset_conf.display() - ) - })?; return Ok(()); } - let file = File::open(self.keyset_conf.clone()).map_err(|e| { - format!( - "unable to open config file {}: {e}", - self.keyset_conf.display() - ) - })?; - let ksc: KeySetConfig = serde_json::from_reader(file) + let config_file = write_locked_file(&self.keyset_conf)?; + + let ksc: KeySetConfig = serde_json::from_reader(&config_file) .map_err(|e| format!("error loading {:?}: {e}\n", self.keyset_conf))?; let file = File::open(ksc.state_file.clone()).map_err(|e| { format!( @@ -744,6 +747,7 @@ impl Keyset { config_changed: false, state_changed: false, run_update_ds_command: false, + _locked_config_file: Some(config_file), #[cfg(feature = "kmip")] pools: HashMap::new(), }; @@ -1466,31 +1470,10 @@ impl Keyset { ws.state_changed = true; } if ws.config_changed { - let json = serde_json::to_string_pretty(&ws.config).expect("should not fail"); - let mut file = File::create(&self.keyset_conf).map_err(|e| { - format!("unable to create file {}: {e}", self.keyset_conf.display()) - })?; - write!(file, "{json}").map_err(|e| { - format!( - "unable to write to file {}: {e}", - self.keyset_conf.display() - ) - })?; + ws.write_config(&self.keyset_conf)?; } if ws.state_changed { - let json = serde_json::to_string_pretty(&ws.state).expect("should not fail"); - let mut file = File::create(&ws.config.state_file).map_err(|e| { - format!( - "unable to create file {}: {e}", - ws.config.state_file.display() - ) - })?; - write!(file, "{json}").map_err(|e| { - format!( - "unable to write to file {}: {e}", - ws.config.state_file.display() - ) - })?; + ws.write_state()?; } // Now check if we need to run the update_ds_command. Make sure that @@ -3666,6 +3649,65 @@ impl WorkSpace { let new_algs = HashSet::from([self.config.algorithm.to_generate_params().algorithm()]); curr_algs != new_algs } + + /// Write config to a file. + /// + /// First write to a new filename and then rename to make sure that + /// changes are atomic. + fn write_config(&self, keyset_conf: &PathBuf) -> Result<(), Error> { + let json = serde_json::to_string_pretty(&self.config).expect("should not fail"); + let mut conf_file_new = keyset_conf.clone(); + + // It would be nice to use add_extension here, but it is only in + // Rust 1.91.0 and above. Use strings instead. + // if !conf_file_new.add_extension("new") { + // return Err(format!("unable to add extension 'new' to {}", + // keyset_conf.display()).into()); + // } + conf_file_new.as_mut_os_string().push(".new"); + + let mut file = File::create(&conf_file_new) + .map_err(|e| format!("unable to create file {}: {e}", conf_file_new.display()))?; + write!(file, "{json}") + .map_err(|e| format!("unable to write to file {}: {e}", conf_file_new.display()))?; + + rename(&conf_file_new, keyset_conf).map_err(|e| { + format!( + "unable to rename {} to {}: {e}", + conf_file_new.display(), + keyset_conf.display() + ) + })?; + Ok(()) + } + + /// Write state to a file. + /// + /// First write to a new filename and then rename to make sure that + /// changes are atomic. + fn write_state(&self) -> Result<(), Error> { + let json = serde_json::to_string_pretty(&self.state).expect("should not fail"); + let mut state_file_new = self.config.state_file.clone(); + // It would be nice to use add_extension here, but it is only in + // Rust 1.91.0 and above. Use strings instead. + // if !state_file_new.add_extension("new") { + // return Err(format!("unable to add extension 'new' to {}", + // ws.config.state_file.display()).into()); + // } + state_file_new.as_mut_os_string().push(".new"); + let mut file = File::create(&state_file_new) + .map_err(|e| format!("unable to create file {}: {e}", state_file_new.display()))?; + write!(file, "{json}") + .map_err(|e| format!("unable to write to file {}: {e}", state_file_new.display()))?; + rename(&state_file_new, &self.config.state_file).map_err(|e| { + format!( + "unable to rename {} to {}: {e}", + state_file_new.display(), + self.config.state_file.display() + ) + })?; + Ok(()) + } } /// Create CDS and CDNSKEY RRsets. @@ -5564,6 +5606,68 @@ fn show_automatic_roll_state( } } +/// Open filename, get an exclusive lock and return the open file. +/// +/// Assume changes are saved by creating a new file and renaming. After +/// locking the file, the function has to check if the locked file is this +/// the current fine under that name. +fn write_locked_file(filename: &PathBuf) -> Result { + // The config file is updated by writing to a new file and the renaming. + // Might have locked the old file. Check. Try a number of times and + // then give up. Lock contention is expected to be low. + for _try in 0..MAX_FILE_LOCK_TRIES { + // First get the metadata before + // we try to lock. + let md = metadata(filename) + .map_err(|e| format!("unable to get metadata for {}: {e}", filename.display()))?; + let file = File::open(filename) + .map_err(|e| format!("unable to open file {}: {e}", filename.display()))?; + + file.lock_exclusive() + .map_err(|e| format!("unable to lock {}: {e}", filename.display()))?; + + // Get metadata again. + let md2 = metadata(filename) + .map_err(|e| format!("unable to get metadata for {}: {e}", filename.display()))?; + + // Check if nothing changed. Created may not exist on all platforms, + // use a default if we get an error. + if md.created().unwrap_or(UNIX_EPOCH) != md2.created().unwrap_or(UNIX_EPOCH) + || md.modified().map_err(|e| { + format!( + "failed to get the modified time of {}: {e}", + filename.display() + ) + })? != md2.modified().map_err(|e| { + format!( + "failed to get the modified time of {}: {e}", + filename.display() + ) + })? + { + continue; + } + + let file2 = File::open(filename) + .map_err(|e| format!("unable to open file {}: {e}", filename.display()))?; + + let filestat = fstat(file.as_fd()) + .map_err(|e| format!("fstat failed for {}: {e}", filename.display()))?; + let filestat2 = fstat(file2.as_fd()) + .map_err(|e| format!("fstat failed for {}: {e}", filename.display()))?; + if filestat.st_ino != filestat2.st_ino { + todo!(); + } + + return Ok(file); + } + Err(format!( + "unable to lock {} after {MAX_FILE_LOCK_TRIES} tries", + filename.display() + ) + .into()) +} + /* Test for RRSIG check - records before the zone From c53687a5ba94cfb891ca73c1f6d2b5925358504c Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Wed, 19 Nov 2025 11:51:11 +0100 Subject: [PATCH 2/8] Update TODO section. --- src/commands/keyset/cmd.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 0a0f5d8..c10c9dd 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -3,12 +3,6 @@ #![warn(clippy::missing_docs_in_private_items)] // TODO: -// - file locking -// - update the state file atomically by writing to a different file and -// then renaming. This helps downstream users. -// - option: overwrite the config file. This is risky but simplifies locking. -// - option: update the config file atomically. This makes locking more -// complex. // - add a autoremove_delay option. Delete stale keys only some time after // their withdrawn times. // - move dnskey_rrset, cds_rrset, ns_rrset into apex_extra (ds_rrset remains). @@ -16,6 +10,7 @@ // the apex. // - add a -v option to keyset. Remove the -v option from status. Add // verbose output for creation and deletion of keys. +// - add a way for status and other read commands to by-pass the lock. use crate::env::Env; use crate::error::Error; From c84e21cd67b4b09cfef811b1c15c91087a8df3d9 Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Wed, 26 Nov 2025 12:23:36 +0100 Subject: [PATCH 3/8] Cargo fmt. --- src/commands/keyset/cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 54f7b1f..a6ceb1e 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -48,13 +48,13 @@ use domain::resolv::StubResolver; #[cfg(feature = "kmip")] use domain::utils::base32::encode_string_hex; use domain::zonefile::inplace::{Entry, Zonefile}; -use fs2::FileExt; #[cfg(feature = "kmip")] use domain_kmip as kmip; #[cfg(feature = "kmip")] use domain_kmip::dep::kmip::client::pool::SyncConnPool; #[cfg(feature = "kmip")] use domain_kmip::KeyUrl; +use fs2::FileExt; use futures::future::join_all; use jiff::{Span, SpanRelativeTo}; use nix::sys::stat::fstat; From c21a9dbf1810c0704d12ff7e5a430c0142ef96fe Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Wed, 26 Nov 2025 16:53:33 +0100 Subject: [PATCH 4/8] Switch to same-file. --- Cargo.lock | 52 ++++++++++++++++++++++++++++++++++++++ Cargo.toml | 2 +- src/commands/keyset/cmd.rs | 51 ++++++++++--------------------------- 3 files changed, 67 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd3192d..e7a6ddf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -320,6 +320,7 @@ dependencies = [ "const_format", "domain", "domain-kmip", + "fs2", "futures", "indenter", "jiff", @@ -332,6 +333,7 @@ dependencies = [ "rayon", "regex", "ring", + "same-file", "serde", "serde_json", "tempfile", @@ -499,6 +501,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "futures" version = "0.3.31" @@ -1392,6 +1404,15 @@ version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "scheduled-thread-pool" version = "0.2.7" @@ -1898,6 +1919,37 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys 0.61.2", +] + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-core" version = "0.62.2" diff --git a/Cargo.toml b/Cargo.toml index 56af10b..4a2261a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,7 +70,7 @@ tracing-subscriber = "0.3.19" url = "2.5.4" futures = "0.3.31" fs2 = "0.4.3" -nix = { version = "0.30.1", features = ["fs"] } +same-file = "1.0.6" [dev-dependencies] const_format = " 0.2.33" diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index a6ceb1e..c60e5e7 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -57,17 +57,16 @@ use domain_kmip::KeyUrl; use fs2::FileExt; use futures::future::join_all; use jiff::{Span, SpanRelativeTo}; -use nix::sys::stat::fstat; +use same_file::Handle; use serde::{Deserialize, Serialize}; use std::cmp::max; use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::From; use std::ffi::OsStr; use std::fmt::{Debug, Display, Formatter}; -use std::fs::{create_dir_all, metadata, remove_file, rename, File}; +use std::fs::{create_dir_all, remove_file, rename, File}; use std::io::{self, Write}; use std::net::{IpAddr, SocketAddr}; -use std::os::fd::AsFd; use std::path::{absolute, Path, PathBuf}; use std::process::Command; use std::sync::Mutex; @@ -5638,49 +5637,27 @@ fn write_locked_file(filename: &PathBuf) -> Result { // Might have locked the old file. Check. Try a number of times and // then give up. Lock contention is expected to be low. for _try in 0..MAX_FILE_LOCK_TRIES { - // First get the metadata before - // we try to lock. - let md = metadata(filename) - .map_err(|e| format!("unable to get metadata for {}: {e}", filename.display()))?; let file = File::open(filename) .map_err(|e| format!("unable to open file {}: {e}", filename.display()))?; file.lock_exclusive() .map_err(|e| format!("unable to lock {}: {e}", filename.display()))?; - // Get metadata again. - let md2 = metadata(filename) - .map_err(|e| format!("unable to get metadata for {}: {e}", filename.display()))?; + let file_clone = file + .try_clone() + .map_err(|e| format!("unable to clone locked file {}: {e}", filename.display()))?; + let locked_file_handle = Handle::from_file(file_clone).map_err(|e| { + format!( + "Unable to get handle from locked file {}: {e}", + filename.display() + ) + })?; + let current_file_handle = Handle::from_path(filename) + .map_err(|e| format!("Unable to get handle from file {}: {e}", filename.display()))?; - // Check if nothing changed. Created may not exist on all platforms, - // use a default if we get an error. - if md.created().unwrap_or(UNIX_EPOCH) != md2.created().unwrap_or(UNIX_EPOCH) - || md.modified().map_err(|e| { - format!( - "failed to get the modified time of {}: {e}", - filename.display() - ) - })? != md2.modified().map_err(|e| { - format!( - "failed to get the modified time of {}: {e}", - filename.display() - ) - })? - { + if locked_file_handle != current_file_handle { continue; } - - let file2 = File::open(filename) - .map_err(|e| format!("unable to open file {}: {e}", filename.display()))?; - - let filestat = fstat(file.as_fd()) - .map_err(|e| format!("fstat failed for {}: {e}", filename.display()))?; - let filestat2 = fstat(file2.as_fd()) - .map_err(|e| format!("fstat failed for {}: {e}", filename.display()))?; - if filestat.st_ino != filestat2.st_ino { - todo!(); - } - return Ok(file); } Err(format!( From 570220ad379add44a184eadf92dbeda7fb086a7f Mon Sep 17 00:00:00 2001 From: Philip-NLnetLabs Date: Mon, 2 Mar 2026 10:56:57 +0100 Subject: [PATCH 5/8] Update src/commands/keyset/cmd.rs Co-authored-by: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> --- src/commands/keyset/cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 2ba59e4..a9a35c1 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -5705,7 +5705,7 @@ fn show_automatic_roll_state( /// locking the file, the function has to check if the locked file is this /// the current fine under that name. fn write_locked_file(filename: &PathBuf) -> Result { - // The config file is updated by writing to a new file and the renaming. + // The config file is updated by writing to a new file and then renaming. // Might have locked the old file. Check. Try a number of times and // then give up. Lock contention is expected to be low. for _try in 0..MAX_FILE_LOCK_TRIES { From 9301b12047cf64f3be2877a000ae35a2042e48e7 Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Mon, 2 Mar 2026 11:03:30 +0100 Subject: [PATCH 6/8] Review feedback. --- src/commands/keyset/cmd.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index a9a35c1..013cfed 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -5702,11 +5702,11 @@ fn show_automatic_roll_state( /// Open filename, get an exclusive lock and return the open file. /// /// Assume changes are saved by creating a new file and renaming. After -/// locking the file, the function has to check if the locked file is this -/// the current fine under that name. +/// locking the file, the function has to check if the locked file is the +/// same as the current file under that name. fn write_locked_file(filename: &PathBuf) -> Result { // The config file is updated by writing to a new file and then renaming. - // Might have locked the old file. Check. Try a number of times and + // We might have locked the old file. Check. Try a number of times and // then give up. Lock contention is expected to be low. for _try in 0..MAX_FILE_LOCK_TRIES { let file = File::open(filename) From 10d2fa669005aeee060caad7ad91fec508c73e53 Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Mon, 2 Mar 2026 11:37:00 +0100 Subject: [PATCH 7/8] Extract common code in write_config and write_state. --- src/commands/keyset/cmd.rs | 52 +++++++++++--------------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 013cfed..0515c58 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -3744,59 +3744,37 @@ impl WorkSpace { } /// Write config to a file. - /// - /// First write to a new filename and then rename to make sure that - /// changes are atomic. fn write_config(&self, keyset_conf: &PathBuf) -> Result<(), Error> { let json = serde_json::to_string_pretty(&self.config).expect("should not fail"); - let mut conf_file_new = keyset_conf.clone(); - - // It would be nice to use add_extension here, but it is only in - // Rust 1.91.0 and above. Use strings instead. - // if !conf_file_new.add_extension("new") { - // return Err(format!("unable to add extension 'new' to {}", - // keyset_conf.display()).into()); - // } - conf_file_new.as_mut_os_string().push(".new"); - - let mut file = File::create(&conf_file_new) - .map_err(|e| format!("unable to create file {}: {e}", conf_file_new.display()))?; - write!(file, "{json}") - .map_err(|e| format!("unable to write to file {}: {e}", conf_file_new.display()))?; - - rename(&conf_file_new, keyset_conf).map_err(|e| { - format!( - "unable to rename {} to {}: {e}", - conf_file_new.display(), - keyset_conf.display() - ) - })?; - Ok(()) + Self::write_to_new_and_rename(&json, keyset_conf) } /// Write state to a file. - /// - /// First write to a new filename and then rename to make sure that - /// changes are atomic. fn write_state(&self) -> Result<(), Error> { let json = serde_json::to_string_pretty(&self.state).expect("should not fail"); - let mut state_file_new = self.config.state_file.clone(); + Self::write_to_new_and_rename(&json, &self.config.state_file) + } + + /// First write to a new filename and then rename to make sure that + /// changes are atomic. + fn write_to_new_and_rename(json: &str, filename: &PathBuf) -> Result<(), Error> { + let mut filename_new = filename.clone(); // It would be nice to use add_extension here, but it is only in // Rust 1.91.0 and above. Use strings instead. // if !state_file_new.add_extension("new") { // return Err(format!("unable to add extension 'new' to {}", // ws.config.state_file.display()).into()); // } - state_file_new.as_mut_os_string().push(".new"); - let mut file = File::create(&state_file_new) - .map_err(|e| format!("unable to create file {}: {e}", state_file_new.display()))?; + filename_new.as_mut_os_string().push(".new"); + let mut file = File::create(&filename_new) + .map_err(|e| format!("unable to create file {}: {e}", filename_new.display()))?; write!(file, "{json}") - .map_err(|e| format!("unable to write to file {}: {e}", state_file_new.display()))?; - rename(&state_file_new, &self.config.state_file).map_err(|e| { + .map_err(|e| format!("unable to write to file {}: {e}", filename_new.display()))?; + rename(&filename_new, filename).map_err(|e| { format!( "unable to rename {} to {}: {e}", - state_file_new.display(), - self.config.state_file.display() + filename_new.display(), + filename.display() ) })?; Ok(()) From 59764425d2036256f539d70168b10b0ea4278c79 Mon Sep 17 00:00:00 2001 From: Philip Homburg Date: Mon, 2 Mar 2026 11:41:49 +0100 Subject: [PATCH 8/8] write_locked_file -> file_with_write_lock --- src/commands/keyset/cmd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/keyset/cmd.rs b/src/commands/keyset/cmd.rs index 0515c58..400c4c4 100644 --- a/src/commands/keyset/cmd.rs +++ b/src/commands/keyset/cmd.rs @@ -736,7 +736,7 @@ impl Keyset { return Ok(()); } - let config_file = write_locked_file(&self.keyset_conf)?; + let config_file = file_with_write_lock(&self.keyset_conf)?; let ksc: KeySetConfig = serde_json::from_reader(&config_file) .map_err(|e| format!("error loading {:?}: {e}\n", self.keyset_conf))?; @@ -5682,7 +5682,7 @@ fn show_automatic_roll_state( /// Assume changes are saved by creating a new file and renaming. After /// locking the file, the function has to check if the locked file is the /// same as the current file under that name. -fn write_locked_file(filename: &PathBuf) -> Result { +fn file_with_write_lock(filename: &PathBuf) -> Result { // The config file is updated by writing to a new file and then renaming. // We might have locked the old file. Check. Try a number of times and // then give up. Lock contention is expected to be low.