diff --git a/src/config.rs b/src/config.rs index a41833d743..84e267ea42 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1134,7 +1134,7 @@ impl StateFile { if !utils::is_file(&self.path) { return Ok(State::default()); } - let content = utils::read_file("state", &self.path)?; + let content = utils::read_locked_file("state", &self.path)?; State::parse(&content).with_context(|| RustupError::ParsingFile { name: "state", path: self.path.clone(), @@ -1142,7 +1142,7 @@ impl StateFile { } fn store(&self, state: &State) -> Result<()> { - utils::write_file("state", &self.path, &state.stringify()?)?; + utils::write_locked_file("state", &self.path, &state.stringify()?)?; Ok(()) } diff --git a/src/settings.rs b/src/settings.rs index 0e07a3746e..c58a0f284c 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -29,7 +29,7 @@ impl SettingsFile { fn write_settings(&self) -> Result<()> { let settings = self.cache.borrow(); - utils::write_file( + utils::write_locked_file( "settings", &self.path, &settings.as_ref().unwrap().stringify()?, @@ -44,7 +44,7 @@ impl SettingsFile { if b.is_none() { drop(b); *self.cache.borrow_mut() = Some(if utils::is_file(&self.path) { - let content = utils::read_file("settings", &self.path)?; + let content = utils::read_locked_file("settings", &self.path)?; Settings::parse(&content).with_context(|| RustupError::ParsingFile { name: "settings", path: self.path.clone(), diff --git a/src/utils/mod.rs b/src/utils/mod.rs index fdb079ccaf..414a5687a4 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -87,6 +87,13 @@ pub fn read_file(name: &'static str, path: &Path) -> Result { }) } +pub(crate) fn read_locked_file(name: &'static str, path: &Path) -> Result { + raw::read_locked_file(path).with_context(|| RustupError::ReadingFile { + name, + path: PathBuf::from(path), + }) +} + pub fn write_file(name: &'static str, path: &Path, contents: &str) -> Result<()> { raw::write_file(path, contents).with_context(|| RustupError::WritingFile { name, @@ -94,6 +101,13 @@ pub fn write_file(name: &'static str, path: &Path, contents: &str) -> Result<()> }) } +pub(crate) fn write_locked_file(name: &'static str, path: &Path, contents: &str) -> Result<()> { + raw::write_locked_file(path, contents).with_context(|| RustupError::WritingFile { + name, + path: PathBuf::from(path), + }) +} + pub(crate) fn append_file(name: &'static str, path: &Path, line: &str) -> Result<()> { raw::append_file(path, line).with_context(|| RustupError::WritingFile { name, diff --git a/src/utils/raw.rs b/src/utils/raw.rs index ba439ae731..70f27f3d42 100644 --- a/src/utils/raw.rs +++ b/src/utils/raw.rs @@ -3,7 +3,7 @@ use std::env; use std::fs; use std::fs::File; use std::io; -use std::io::Write; +use std::io::{Read, Seek, SeekFrom, Write}; use std::path::Path; use std::str; @@ -60,6 +60,21 @@ pub fn path_exists>(path: P) -> bool { fs::metadata(path).is_ok() } +pub(crate) fn read_locked_file(path: &Path) -> io::Result { + let mut file = File::open(path)?; + file.lock_shared()?; + + let size = file + .metadata() + .map(|metadata| usize::try_from(metadata.len()).unwrap_or(usize::MAX)) + .ok(); + let mut contents = String::new(); + contents.try_reserve_exact(size.unwrap_or(0))?; + file.read_to_string(&mut contents)?; + + Ok(contents) +} + pub(crate) fn random_string(length: usize) -> String { const CHARSET: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789_"; let mut rng = rand::rng(); @@ -82,6 +97,24 @@ pub fn write_file(path: &Path, contents: &str) -> io::Result<()> { Ok(()) } +pub(crate) fn write_locked_file(path: &Path, contents: &str) -> io::Result<()> { + let mut file = fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + // Truncation must happen after the exclusive lock is held. + .truncate(false) + .open(path)?; + + file.lock()?; + file.set_len(0)?; + file.seek(SeekFrom::Start(0))?; + Write::write_all(&mut file, contents.as_bytes())?; + file.sync_data()?; + + Ok(()) +} + pub(crate) fn filter_file bool>( src: &Path, dest: &Path, @@ -350,3 +383,20 @@ pub(crate) mod windows { inner(s.as_ref()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn locked_file_round_trip_truncates_previous_contents() -> io::Result<()> { + let temp_dir = tempfile::tempdir()?; + let path = temp_dir.path().join("state.toml"); + + write_locked_file(&path, "last_release_notified_secs = 123456\n")?; + write_locked_file(&path, "version = \"12\"\n")?; + + assert_eq!(read_locked_file(&path)?, "version = \"12\"\n"); + Ok(()) + } +}