Merge pull request #227 from NobodyXu/fix/flock

Fix updating metafiles: Use file lock to fix race condition

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"

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"

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<PathBuf> = OnceCell::new();
 
@@ -50,6 +53,20 @@ 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<Path>) -> io::Result<fs::File> {
+    let path = path.as_ref();
+
+    let mut options = fs::File::options();
+    options.read(true).write(true);
+
+    options
+        .clone()
+        .create_new(true)
+        .open(path)
+        .or_else(|_| options.open(path))
+}
+
 pub async fn await_task<T>(task: tokio::task::JoinHandle<miette::Result<T>>) -> miette::Result<T> {
     match task.await {
         Ok(res) => res,

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<Self> {
+        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<Self> {
+        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
+    }
+}

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,
@@ -11,7 +12,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 {
@@ -27,6 +28,12 @@ impl CratesToml {
         Self::load_from_path(Self::default_path()?)
     }
 
+    pub fn load_from_reader<R: io::Read>(mut reader: R) -> Result<Self, CratesTomlParseError> {
+        let mut vec = Vec::new();
+        reader.read_to_end(&mut vec)?;
+        Ok(toml::from_slice(&vec)?)
+    }
+
     pub fn load_from_path(path: impl AsRef<Path>) -> Result<Self, CratesTomlParseError> {
         let file = fs::read_to_string(path)?;
         Self::from_str(&file)
@@ -40,6 +47,20 @@ impl CratesToml {
         self.write_to_path(Self::default_path()?)
     }
 
+    pub fn write_to_writer<W: io::Write>(&self, mut writer: W) -> Result<(), CratesTomlParseError> {
+        let data = toml::to_vec(&self)?;
+        writer.write_all(&data)?;
+        Ok(())
+    }
+
+    pub fn write_to_file(&self, file: &mut fs::File) -> Result<(), CratesTomlParseError> {
+        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<Path>) -> Result<(), CratesTomlParseError> {
         fs::write(path, &toml::to_vec(&self)?)?;
         Ok(())
@@ -52,17 +73,15 @@ impl CratesToml {
     where
         Iter: IntoIterator<Item = (&'a CrateVersionSource, BTreeSet<String>)>,
     {
-        let mut c1 = match Self::load_from_path(path.as_ref()) {
+        let mut file = FileLock::new_exclusive(create_if_not_exist(path.as_ref())?)?;
-            Ok(c1) => c1,
+        let mut c1 = Self::load_from_reader(&mut *file)?;
-            Err(CratesTomlParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => {
+
-                Self::default()
-            }
-            Err(err) => return Err(err),
-        };
         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(())
     }

src/metafiles/v2.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::{BTreeMap, BTreeSet},
-    fs, io,
+    fs,
+    io::{self, Seek},
     iter::IntoIterator,
     path::{Path, PathBuf},
 };
@@ -10,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 {
@@ -48,9 +49,13 @@ impl Crates2Json {
         Self::load_from_path(Self::default_path()?)
     }
 
+    pub fn load_from_reader<R: io::Read>(reader: R) -> Result<Self, Crates2JsonParseError> {
+        Ok(serde_json::from_reader(reader)?)
+    }
+
     pub fn load_from_path(path: impl AsRef<Path>) -> Result<Self, Crates2JsonParseError> {
         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) {
@@ -61,10 +66,22 @@ impl Crates2Json {
         self.write_to_path(Self::default_path()?)
     }
 
+    pub fn write_to_writer<W: io::Write>(&self, writer: W) -> Result<(), Crates2JsonParseError> {
+        serde_json::to_writer(writer, &self)?;
+        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<Path>) -> Result<(), Crates2JsonParseError> {
         let file = fs::File::create(path.as_ref())?;
-        serde_json::to_writer(file, &self)?;
+        self.write_to_writer(file)
-        Ok(())
     }
 
     pub fn append_to_path<Iter>(
@@ -74,17 +91,15 @@ impl Crates2Json {
     where
         Iter: IntoIterator<Item = (CrateVersionSource, CrateInfo)>,
     {
-        let mut c2 = match Self::load_from_path(path.as_ref()) {
+        let mut file = FileLock::new_exclusive(create_if_not_exist(path.as_ref())?)?;
-            Ok(c2) => c2,
+        let mut c2 = Self::load_from_reader(&mut *file)?;
-            Err(Crates2JsonParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => {
+
-                Self::default()
-            }
-            Err(err) => return Err(err),
-        };
         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(())
     }