improved locking so that file access is done through the lock guard

Author: Heliozoa

tmc-langs-cli/src/config/credentials.rs

@@ -1,7 +1,9 @@
 use crate::Token;
 use anyhow::{Context, Result};
+use file_util::create_file_lock;
 use serde::{Deserialize, Serialize};
-use std::fs::{self, File};
+use std::fs;
+use std::ops::Deref;
 use std::path::PathBuf;
 use tmc_langs_framework::file_util;
 
@@ -29,15 +31,15 @@ impl Credentials {
             return Ok(None);
         }
 
-        file_util::lock!(&credentials_path);
-
-        let file = File::open(&credentials_path).with_context(|| {
+        let mut credentials_file = file_util::open_file_lock(&credentials_path)?;
+        let guard = credentials_file.lock().with_context(|| {
             format!(
-                "Failed to read credentials file at {}",
+                "Failed to lock credentials file at {}",
                 credentials_path.display()
             )
         })?;
-        match serde_json::from_reader(file) {
+
+        match serde_json::from_reader(guard.deref()) {
             Ok(token) => Ok(Some(Credentials {
                 path: credentials_path,
                 token,
@@ -64,21 +66,16 @@ impl Credentials {
     pub fn save(client_name: &str, token: Token) -> Result<()> {
         let credentials_path = Self::get_credentials_path(client_name)?;
 
-        if credentials_path.exists() {
-            // waiting for any other process working with the file to finish
-            file_util::lock!(&credentials_path);
-        }
-
         if let Some(p) = credentials_path.parent() {
             fs::create_dir_all(p)
                 .with_context(|| format!("Failed to create directory {}", p.display()))?;
         }
-        let credentials_file = File::create(&credentials_path)
+        let mut credentials_file = create_file_lock(&credentials_path)
             .with_context(|| format!("Failed to create file at {}", credentials_path.display()))?;
-        file_util::lock!(&credentials_path);
+        let guard = credentials_file.lock()?;
 
         // write token
-        if let Err(e) = serde_json::to_writer(credentials_file, &token) {
+        if let Err(e) = serde_json::to_writer(guard.deref(), &token) {
             // failed to write token, removing credentials file
             fs::remove_file(&credentials_path).with_context(|| {
                 format!(

tmc-langs-cli/src/config/tmc_config.rs

@@ -2,12 +2,9 @@
 
 use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
-use std::io::Write;
+use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
-use std::{
-    borrow::Cow,
-    fs::{self, File},
-};
+use std::{borrow::Cow, fs};
 use tmc_langs_framework::file_util;
 use toml::{value::Table, Value};
 
@@ -66,28 +63,30 @@ impl TmcConfig {
 
     pub fn save(self, client_name: &str) -> Result<()> {
         let path = Self::get_location(client_name)?;
-        file_util::lock!(&path);
+        let mut lock = file_util::create_file_lock(&path)?;
+        let mut guard = lock.lock()?;
 
         let toml = toml::to_string_pretty(&self).context("Failed to serialize HashMap")?;
-        fs::write(&path, toml.as_bytes())
+        guard
+            .write_all(toml.as_bytes())
             .with_context(|| format!("Failed to write TOML to {}", path.display()))?;
         Ok(())
     }
 
     pub fn reset(client_name: &str) -> Result<()> {
         let path = Self::get_location(client_name)?;
-        file_util::lock!(&path);
-
-        Self::init_at(client_name, &path)?;
+        Self::init_at(client_name, &path)?; // init locks the file
         Ok(())
     }
 
     pub fn load(client_name: &str) -> Result<TmcConfig> {
         let path = Self::get_location(client_name)?;
-        file_util::lock!(&path);
+        let mut lock = file_util::open_file_lock(&path)?;
+        let mut guard = lock.lock()?;
 
-        let config = match fs::read(&path) {
-            Ok(bytes) => match toml::from_slice(&bytes) {
+        let mut buf = vec![];
+        let config = match guard.read_to_end(&mut buf) {
+            Ok(_) => match toml::from_slice(&buf) {
                 Ok(config) => Ok(config),
                 Err(_) => {
                     log::error!(
@@ -112,10 +111,9 @@ impl TmcConfig {
 
     // initializes the default configuration file at the given path
     fn init_at(client_name: &str, path: &Path) -> Result<TmcConfig> {
-        file_util::lock!(path);
-
-        let mut file = File::create(&path)
+        let mut lock = file_util::create_file_lock(path)
             .with_context(|| format!("Failed to create new config file at {}", path.display()))?;
+        let mut guard = lock.lock()?;
 
         let default_project_dir = dirs::data_local_dir()
             .context("Failed to find local data directory")?
@@ -134,7 +132,8 @@ impl TmcConfig {
         };
 
         let toml = toml::to_string_pretty(&config).context("Failed to serialize config")?;
-        file.write_all(toml.as_bytes())
+        guard
+            .write_all(toml.as_bytes())
             .with_context(|| format!("Failed to write default config to {}", path.display()))?;
         Ok(config)
     }

tmc-langs-framework/src/file_util.rs

@@ -1,6 +1,7 @@
 //! Various utility functions, primarily wrapping the standard library's IO and filesystem functions
 
 use crate::error::FileIo;
+use fd_lock::FdLock;
 use std::fs::{self, File};
 use std::io::{Read, Write};
 use std::path::Path;
@@ -25,6 +26,13 @@ pub fn open_file<P: AsRef<Path>>(path: P) -> Result<File, FileIo> {
     File::open(path).map_err(|e| FileIo::FileOpen(path.to_path_buf(), e))
 }
 
+/// Does not work on directories on Windows.
+pub fn open_file_lock<P: AsRef<Path>>(path: P) -> Result<FdLock<File>, FileIo> {
+    let file = open_file(path)?;
+    let lock = FdLock::new(file);
+    Ok(lock)
+}
+
 pub fn read_file<P: AsRef<Path>>(path: P) -> Result<Vec<u8>, FileIo> {
     let path = path.as_ref();
     let mut file = open_file(path)?;
@@ -50,6 +58,17 @@ pub fn create_file<P: AsRef<Path>>(path: P) -> Result<File, FileIo> {
     File::create(path).map_err(|e| FileIo::FileCreate(path.to_path_buf(), e))
 }
 
+pub fn create_file_lock<P: AsRef<Path>>(path: P) -> Result<FdLock<File>, FileIo> {
+    if let Ok(existing) = open_file(&path) {
+        // wait for an existing process to be done with the file before rewriting
+        let mut lock = FdLock::new(existing);
+        lock.lock().unwrap();
+    }
+    let file = create_file(path)?;
+    let lock = FdLock::new(file);
+    Ok(lock)
+}
+
 pub fn remove_file<P: AsRef<Path>>(path: P) -> Result<(), FileIo> {
     let path = path.as_ref();
     fs::remove_file(path).map_err(|e| FileIo::FileRemove(path.to_path_buf(), e))