From 751cf47716b254be8ea032055259f99fae6c099c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 19:42:33 +1000 Subject: [PATCH 01/14] Add new dep fs4 v0.6.2 Signed-off-by: Jiahao XU --- Cargo.lock | 38 ++++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + 2 files changed, 39 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 4ddc98b4..439ec66f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -109,6 +109,7 @@ dependencies = [ "dirs", "env_logger", "flate2", + "fs4", "futures-util", "guess_host_triple", "home", @@ -369,6 +370,17 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs4" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9813c3dc174931eff4bd78609debba56465b7c1da888576d21636b601a46790" +dependencies = [ + "libc", + "rustix", + "winapi", +] + [[package]] name = "futures" version = "0.3.21" @@ -637,6 +649,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "io-lifetimes" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24c3f4eff5495aee4c0399d7b6a0dc2b6e81be84242ffbfcf253ebacccc1d0cb" + [[package]] name = "ipnet" version = "2.5.0" @@ -704,6 +722,12 @@ dependencies = [ "libc", ] +[[package]] +name = "linux-raw-sys" +version = "0.0.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4d2456c373231a208ad294c33dc5bff30051eafd954cd4caae83a712b12854d" + [[package]] name = "log" version = "0.4.17" @@ -1025,6 +1049,20 @@ dependencies = [ "winapi", ] +[[package]] +name = "rustix" +version = "0.35.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d51cc38aa10f6bbb377ed28197aa052aa4e2b762c22be9d3153d01822587e787" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "rustls" version = "0.20.6" diff --git a/Cargo.toml b/Cargo.toml index 883a9e72..5706941e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ clap = { version = "3.2.12", features = ["derive"] } crates_io_api = { version = "0.8.0", default-features = false, features = ["rustls"] } dirs = "4.0.0" flate2 = { version = "1.0.24", features = ["zlib-ng"], default-features = false } +fs4 = "0.6.2" futures-util = { version = "0.3.21", default-features = false } home = "0.5.3" jobserver = "0.1.24" From 9e45ba103224554b757c77cc0b2dc93823ed62b7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 19:48:37 +1000 Subject: [PATCH 02/14] Impl new RAII type `helpers::flock::FileLock` that locks a file exclusive or shared Signed-off-by: Jiahao XU --- src/helpers.rs | 3 +++ src/helpers/flock.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 src/helpers/flock.rs diff --git a/src/helpers.rs b/src/helpers.rs index a1ef3c5f..55593da3 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -42,6 +42,9 @@ pub use tls_version::TLSVersion; mod crate_name; pub use crate_name::CrateName; +mod flock; +pub use flock::FileLock; + pub fn cargo_home() -> Result<&'static Path, io::Error> { static CARGO_HOME: OnceCell = OnceCell::new(); diff --git a/src/helpers/flock.rs b/src/helpers/flock.rs new file mode 100644 index 00000000..2c0707b5 --- /dev/null +++ b/src/helpers/flock.rs @@ -0,0 +1,45 @@ +use std::fs::File; +use std::io; +use std::ops; + +use fs4::FileExt; + +#[derive(Debug)] +pub struct FileLock(File); + +impl FileLock { + /// NOTE that this function blocks, so it cannot + /// be called in async context. + pub fn new_exclusive(file: File) -> io::Result { + file.lock_exclusive()?; + + Ok(Self(file)) + } + + /// NOTE that this function blocks, so it cannot + /// be called in async context. + pub fn new_shared(file: File) -> io::Result { + file.lock_shared()?; + + Ok(Self(file)) + } +} + +impl Drop for FileLock { + fn drop(&mut self) { + let _ = self.unlock(); + } +} + +impl ops::Deref for FileLock { + type Target = File; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl ops::DerefMut for FileLock { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} From 565b404dcee055e692a3ab0045731b652f99f377 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 20:15:03 +1000 Subject: [PATCH 03/14] Impl fn `CratesToml::load_from_reader` Signed-off-by: Jiahao XU --- src/metafiles/v1.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/metafiles/v1.rs b/src/metafiles/v1.rs index e356ec60..5ef58a34 100644 --- a/src/metafiles/v1.rs +++ b/src/metafiles/v1.rs @@ -27,6 +27,12 @@ impl CratesToml { Self::load_from_path(Self::default_path()?) } + pub fn load_from_reader(mut reader: R) -> Result { + let mut vec = Vec::new(); + reader.read_to_end(&mut vec)?; + Ok(toml::from_slice(&vec)?) + } + pub fn load_from_path(path: impl AsRef) -> Result { let file = fs::read_to_string(path)?; Self::from_str(&file) From 7311f77f29e5a404b791db9514169d13a255f92b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 20:19:04 +1000 Subject: [PATCH 04/14] Impl new fn `CratesToml::write_to_writer` Signed-off-by: Jiahao XU --- src/metafiles/v1.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/metafiles/v1.rs b/src/metafiles/v1.rs index 5ef58a34..0401a82a 100644 --- a/src/metafiles/v1.rs +++ b/src/metafiles/v1.rs @@ -46,6 +46,15 @@ impl CratesToml { self.write_to_path(Self::default_path()?) } + pub fn write_to_writer( + &self, + mut writer: W, + ) -> Result { + let data = toml::to_vec(&self)?; + writer.write_all(&data)?; + Ok(data.len().try_into().unwrap()) + } + pub fn write_to_path(&self, path: impl AsRef) -> Result<(), CratesTomlParseError> { fs::write(path, &toml::to_vec(&self)?)?; Ok(()) From 05115641ff668be4268cd06541a8e5c23efa893b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:12:16 +1000 Subject: [PATCH 05/14] Add new fn `helpers::create_if_not_exist` Signed-off-by: Jiahao XU --- src/helpers.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 55593da3..6c209738 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -53,6 +53,30 @@ pub fn cargo_home() -> Result<&'static Path, io::Error> { .map(ops::Deref::deref) } +pub fn create_if_not_exist(path: impl AsRef) -> io::Result { + let mut options = fs::File::options(); + options.create_new(true); + + for _ in 0..3 { + match options.open(path.as_ref()) { + Ok(file) => return Ok(file), + Err(io_err) if io_err.kind() == io::ErrorKind::AlreadyExists => { + match fs::File::open(path.as_ref()) { + Ok(file) => return Ok(file), + Err(io_err) if io_err.kind() == io::ErrorKind::NotFound => (), + Err(err) => return Err(err), + } + } + Err(err) => return Err(err), + } + } + + let errmsg = "We tried to open this file three times but all attempts have failed + with creation returns AlreadyExists and opening returns NotFound"; + + Err(io::Error::new(io::ErrorKind::TimedOut, errmsg)) +} + pub async fn await_task(task: tokio::task::JoinHandle>) -> miette::Result { match task.await { Ok(res) => res, From e1b6fb85aa963532fa2b2fc1dbf7364033cb17ba Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:22:18 +1000 Subject: [PATCH 06/14] Add new fn `CratesToml::write_to_file` Signed-off-by: Jiahao XU --- src/metafiles/v1.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/metafiles/v1.rs b/src/metafiles/v1.rs index 0401a82a..596b9591 100644 --- a/src/metafiles/v1.rs +++ b/src/metafiles/v1.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use super::CrateVersionSource; -use crate::cargo_home; +use crate::{cargo_home, create_if_not_exist, FileLock}; #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct CratesToml { @@ -55,6 +55,13 @@ impl CratesToml { Ok(data.len().try_into().unwrap()) } + pub fn write_to_file(&self, file: &mut fs::File) -> Result<(), CratesTomlParseError> { + let cnt = self.write_to_writer(&mut *file)?; + file.set_len(cnt)?; + + Ok(()) + } + pub fn write_to_path(&self, path: impl AsRef) -> Result<(), CratesTomlParseError> { fs::write(path, &toml::to_vec(&self)?)?; Ok(()) From d7bd96660e3fe2cd31765d380887237135698822 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:22:31 +1000 Subject: [PATCH 07/14] Fix `CratesToml::append_to_path`: Lock file to avoid race condition Signed-off-by: Jiahao XU --- src/metafiles/v1.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/metafiles/v1.rs b/src/metafiles/v1.rs index 596b9591..79126091 100644 --- a/src/metafiles/v1.rs +++ b/src/metafiles/v1.rs @@ -1,6 +1,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, - fs, io, + fs, + io::{self, Seek}, iter::IntoIterator, path::{Path, PathBuf}, str::FromStr, @@ -74,17 +75,15 @@ impl CratesToml { where Iter: IntoIterator)>, { - let mut c1 = match Self::load_from_path(path.as_ref()) { - Ok(c1) => c1, - Err(CratesTomlParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => { - Self::default() - } - Err(err) => return Err(err), - }; + let mut file = FileLock::new_exclusive(create_if_not_exist(path.as_ref())?)?; + let mut c1 = Self::load_from_reader(&mut *file)?; + for (cvs, bins) in iter { c1.insert(cvs, bins); } - c1.write_to_path(path.as_ref())?; + + file.rewind()?; + c1.write_to_file(&mut *file)?; Ok(()) } From 09d210bf6261bddb25e23c7aff9630cd4a718541 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:32:45 +1000 Subject: [PATCH 08/14] Simplify `helpers::create_if_not_exist` implementation Signed-off-by: Jiahao XU --- src/helpers.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 6c209738..d240772e 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -54,27 +54,12 @@ pub fn cargo_home() -> Result<&'static Path, io::Error> { } pub fn create_if_not_exist(path: impl AsRef) -> io::Result { - let mut options = fs::File::options(); - options.create_new(true); + let path = path.as_ref(); - for _ in 0..3 { - match options.open(path.as_ref()) { - Ok(file) => return Ok(file), - Err(io_err) if io_err.kind() == io::ErrorKind::AlreadyExists => { - match fs::File::open(path.as_ref()) { - Ok(file) => return Ok(file), - Err(io_err) if io_err.kind() == io::ErrorKind::NotFound => (), - Err(err) => return Err(err), - } - } - Err(err) => return Err(err), - } - } - - let errmsg = "We tried to open this file three times but all attempts have failed - with creation returns AlreadyExists and opening returns NotFound"; - - Err(io::Error::new(io::ErrorKind::TimedOut, errmsg)) + fs::File::options() + .create_new(true) + .open(path) + .or_else(|_| fs::File::open(path)) } pub async fn await_task(task: tokio::task::JoinHandle>) -> miette::Result { From d9fe7bfaf42453e3f083aaae0bbf3b96c1ea695b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:38:10 +1000 Subject: [PATCH 09/14] Fix bug in `helpers::create_if_not_exist` Returned `File` must be both readable and writable. Signed-off-by: Jiahao XU --- src/helpers.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index d240772e..5118722a 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -53,13 +53,18 @@ pub fn cargo_home() -> Result<&'static Path, io::Error> { .map(ops::Deref::deref) } +/// Returned file is readable and writable. pub fn create_if_not_exist(path: impl AsRef) -> io::Result { let path = path.as_ref(); - fs::File::options() + let mut options = fs::File::options(); + options.read(true).write(true); + + options + .clone() .create_new(true) .open(path) - .or_else(|_| fs::File::open(path)) + .or_else(|_| options.open(path)) } pub async fn await_task(task: tokio::task::JoinHandle>) -> miette::Result { From 15e2213225975280c32db9720e08a8fe45cd46f7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:44:28 +1000 Subject: [PATCH 10/14] Add new fn `Crates2Json::load_from_reader` Signed-off-by: Jiahao XU --- src/metafiles/v2.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/metafiles/v2.rs b/src/metafiles/v2.rs index b6c8cc58..6e7bbd09 100644 --- a/src/metafiles/v2.rs +++ b/src/metafiles/v2.rs @@ -48,9 +48,13 @@ impl Crates2Json { Self::load_from_path(Self::default_path()?) } + pub fn load_from_reader(reader: R) -> Result { + Ok(serde_json::from_reader(reader)?) + } + pub fn load_from_path(path: impl AsRef) -> Result { let file = fs::File::open(path.as_ref())?; - Ok(serde_json::from_reader(file)?) + Self::load_from_reader(file) } pub fn insert(&mut self, cvs: &CrateVersionSource, info: CrateInfo) { From 1766b925474e2960fe79274214405aba8865dec4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:45:55 +1000 Subject: [PATCH 11/14] Add new fn `Crates2Json::write_to_writer` Signed-off-by: Jiahao XU --- src/metafiles/v2.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/metafiles/v2.rs b/src/metafiles/v2.rs index 6e7bbd09..3b8d681a 100644 --- a/src/metafiles/v2.rs +++ b/src/metafiles/v2.rs @@ -65,10 +65,14 @@ impl Crates2Json { self.write_to_path(Self::default_path()?) } + pub fn write_to_writer(&self, writer: W) -> Result<(), Crates2JsonParseError> { + serde_json::to_writer(writer, &self)?; + Ok(()) + } + pub fn write_to_path(&self, path: impl AsRef) -> Result<(), Crates2JsonParseError> { let file = fs::File::create(path.as_ref())?; - serde_json::to_writer(file, &self)?; - Ok(()) + self.write_to_writer(file) } pub fn append_to_path( From d432d54c28c759e04c6acc7d92a1e691e9194dd4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:48:54 +1000 Subject: [PATCH 12/14] Add new fn `Crates2Json::write_to_file` Signed-off-by: Jiahao XU --- src/metafiles/v2.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/metafiles/v2.rs b/src/metafiles/v2.rs index 3b8d681a..213461c3 100644 --- a/src/metafiles/v2.rs +++ b/src/metafiles/v2.rs @@ -1,6 +1,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, - fs, io, + fs, + io::{self, Seek}, iter::IntoIterator, path::{Path, PathBuf}, }; @@ -70,6 +71,14 @@ impl Crates2Json { Ok(()) } + pub fn write_to_file(&self, file: &mut fs::File) -> Result<(), Crates2JsonParseError> { + self.write_to_writer(&mut *file)?; + let pos = file.stream_position()?; + file.set_len(pos)?; + + Ok(()) + } + pub fn write_to_path(&self, path: impl AsRef) -> Result<(), Crates2JsonParseError> { let file = fs::File::create(path.as_ref())?; self.write_to_writer(file) From 0a753f3e4bad0aba4d7b7b351382eda5e54f1022 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:50:58 +1000 Subject: [PATCH 13/14] Fix `Crates2Json::append_to_path`: Use file lock to fix race condition. Signed-off-by: Jiahao XU --- src/metafiles/v2.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/metafiles/v2.rs b/src/metafiles/v2.rs index 213461c3..1db6fb42 100644 --- a/src/metafiles/v2.rs +++ b/src/metafiles/v2.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use super::CrateVersionSource; -use crate::cargo_home; +use crate::{cargo_home, create_if_not_exist, FileLock}; #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct Crates2Json { @@ -91,17 +91,15 @@ impl Crates2Json { where Iter: IntoIterator, { - let mut c2 = match Self::load_from_path(path.as_ref()) { - Ok(c2) => c2, - Err(Crates2JsonParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => { - Self::default() - } - Err(err) => return Err(err), - }; + let mut file = FileLock::new_exclusive(create_if_not_exist(path.as_ref())?)?; + let mut c2 = Self::load_from_reader(&mut *file)?; + for (cvs, info) in iter { c2.insert(&cvs, info); } - c2.write_to_path(path.as_ref())?; + + file.rewind()?; + c2.write_to_file(&mut *file)?; Ok(()) } From 6ce48419b6e09d32c39410fffb3d532251ff5867 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 22 Jul 2022 22:55:44 +1000 Subject: [PATCH 14/14] Fix `CratesToml::write_to_{writer, file}` Make them consistent with `Crates2Json::write_to_{writer, file}` Signed-off-by: Jiahao XU --- src/metafiles/v1.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/metafiles/v1.rs b/src/metafiles/v1.rs index 79126091..def69db6 100644 --- a/src/metafiles/v1.rs +++ b/src/metafiles/v1.rs @@ -47,18 +47,16 @@ impl CratesToml { self.write_to_path(Self::default_path()?) } - pub fn write_to_writer( - &self, - mut writer: W, - ) -> Result { + pub fn write_to_writer(&self, mut writer: W) -> Result<(), CratesTomlParseError> { let data = toml::to_vec(&self)?; writer.write_all(&data)?; - Ok(data.len().try_into().unwrap()) + Ok(()) } pub fn write_to_file(&self, file: &mut fs::File) -> Result<(), CratesTomlParseError> { - let cnt = self.write_to_writer(&mut *file)?; - file.set_len(cnt)?; + self.write_to_writer(&mut *file)?; + let pos = file.stream_position()?; + file.set_len(pos)?; Ok(()) }