Skip to content

Commit

Permalink
Implement Pid file locking (#5676)
Browse files Browse the repository at this point in the history
## Description

Fixes #5633

Added functionality to create and manage a lock file containing the
process ID (pid) of the running instance of the software. This mechanism
prevents multiple instances of the software from running simultaneously
by checking the existence and content of the lock file. If the lock file
exists and contains a valid pid, the struct will error gracefully to
avoid conflicts. If the lock file is missing or contains an invalid pid,
the struct will proceed by removing the file. This ensures that only one
instance of the software can run at a time and it avoids stale locking
to prevent future instances


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
crodas authored Mar 6, 2024
1 parent c24d731 commit c1c2501
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 55 deletions.
9 changes: 5 additions & 4 deletions forc-plugins/forc-fmt/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use forc_pkg::{
manifest::{GenericManifestFile, ManifestFile},
WorkspaceManifestFile,
};
use forc_util::fs_locking::PidFileLocking;
use prettydiff::{basic::DiffOp, diff_lines};
use std::{
default::Default,
Expand Down Expand Up @@ -51,7 +52,8 @@ pub struct App {
pub path: Option<String>,
#[clap(short, long)]
/// Formats a single .sw file with the default settings.
/// If not specified, current working directory will be formatted using a Forc.toml configuration.
/// If not specified, current working directory will be formatted using a Forc.toml
/// configuration.
pub file: Option<String>,
}

Expand Down Expand Up @@ -109,9 +111,8 @@ fn run() -> Result<()> {
/// with unsaved changes.
///
/// Returns `true` if a corresponding "dirty" flag file exists, `false` otherwise.
fn is_file_dirty(path: &Path) -> bool {
let dirty_file_path = forc_util::is_dirty_path(path);
dirty_file_path.exists()
fn is_file_dirty<X: AsRef<Path>>(path: X) -> bool {
PidFileLocking::lsp(path.as_ref()).is_locked()
}

/// Recursively get a Vec<PathBuf> of subdirectories that contains a Forc.toml.
Expand Down
201 changes: 201 additions & 0 deletions forc-util/src/fs_locking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
use crate::{hash_path, user_forc_directory};
use std::{
fs::{create_dir_all, remove_file, File},
io::{self, Read, Write},
path::{Path, PathBuf},
};

/// Very simple AdvisoryPathMutex class
///
/// The goal of this struct is to signal other processes that a path is being used by another
/// process exclusively.
///
/// This struct will self-heal if the process that locked the file is no longer running.
pub struct PidFileLocking(PathBuf);

impl PidFileLocking {
pub fn new<X: AsRef<Path>, Y: AsRef<Path>>(
filename: X,
dir: Y,
extension: &str,
) -> PidFileLocking {
let file_name = hash_path(filename);
Self(
user_forc_directory()
.join(dir)
.join(file_name)
.with_extension(extension),
)
}

/// Create a new PidFileLocking instance that is shared between the LSP and any other process
/// that may want to update the file and needs to wait for the LSP to finish (like forc-fmt)
pub fn lsp<X: AsRef<Path>>(filename: X) -> PidFileLocking {
Self::new(filename, ".lsp-locks", "lock")
}

/// Checks if the given pid is active
#[cfg(not(target = "windows"))]
fn is_pid_active(pid: usize) -> bool {
// Not using sysinfo here because it has compatibility issues with fuel.nix
// https://github.com/FuelLabs/fuel.nix/issues/64
use std::process::Command;
let output = Command::new("ps")
.arg("-p")
.arg(pid.to_string())
.output()
.expect("Failed to execute ps command");

let output_str = String::from_utf8_lossy(&output.stdout);
output_str.contains(&format!("{} ", pid))
}

#[cfg(target = "windows")]
fn is_pid_active(pid: usize) -> bool {
// Not using sysinfo here because it has compatibility issues with fuel.nix
// https://github.com/FuelLabs/fuel.nix/issues/64
use std::process::Command;
let output = Command::new("tasklist")
.arg("/FI")
.arg(format!("PID eq {}", pid))
.output()
.expect("Failed to execute tasklist command");

let output_str = String::from_utf8_lossy(&output.stdout);
// Check if the output contains the PID, indicating the process is active
output_str.contains(&format!("{}", pid))
}

/// Removes the lock file if it is not locked or the process that locked it is no longer active
pub fn release(&self) -> io::Result<()> {
if self.is_locked() {
Err(io::Error::new(
std::io::ErrorKind::Other,
"Cannot remove a dirty lock file, it is locked by another process",
))
} else {
self.remove_file()?;
Ok(())
}
}

fn remove_file(&self) -> io::Result<()> {
match remove_file(&self.0) {
Err(e) => {
if e.kind() != std::io::ErrorKind::NotFound {
return Err(e);
}
Ok(())
}
_ => Ok(()),
}
}

/// Returns the PID of the owner of the current lock. If the PID is not longer active the lock
/// file will be removed
pub fn get_locker_pid(&self) -> Option<usize> {
let fs = File::open(&self.0);
if let Ok(mut file) = fs {
let mut contents = String::new();
file.read_to_string(&mut contents).ok();
drop(file);
if let Ok(pid) = contents.trim().parse::<usize>() {
return if Self::is_pid_active(pid) {
Some(pid)
} else {
let _ = self.remove_file();
None
};
}
}
None
}

/// Checks if the current path is owned by any other process. This will return false if there is
/// no lock file or the current process is the owner of the lock file
pub fn is_locked(&self) -> bool {
self.get_locker_pid()
.map(|pid| pid != (std::process::id() as usize))
.unwrap_or_default()
}

/// Locks the given filepath if it is not already locked
pub fn lock(&self) -> io::Result<()> {
self.release()?;
if let Some(dir) = self.0.parent() {
// Ensure the directory exists
create_dir_all(dir)?;
}

let mut fs = File::create(&self.0)?;
fs.write_all(std::process::id().to_string().as_bytes())?;
fs.sync_all()?;
fs.flush()?;
Ok(())
}
}

#[cfg(test)]
mod test {
use super::PidFileLocking;
use std::{
fs::{metadata, File},
io::{ErrorKind, Write},
os::unix::fs::MetadataExt,
};

#[test]
fn test_fs_locking_same_process() {
let x = PidFileLocking::lsp("test");
assert!(!x.is_locked()); // checks the non-existance of the lock (therefore it is not locked)
assert!(x.lock().is_ok());
// The current process is locking "test"
let x = PidFileLocking::lsp("test");
assert!(!x.is_locked());
}

#[test]
fn test_legacy() {
// tests against an empty file (as legacy were creating this files)
let x = PidFileLocking::lsp("legacy");
assert!(x.lock().is_ok());
// lock file exists,
assert!(metadata(&x.0).is_ok());

// simulate a stale lock file from legacy (which should be empty)
let _ = File::create(&x.0).unwrap();
assert_eq!(metadata(&x.0).unwrap().size(), 0);

let x = PidFileLocking::lsp("legacy");
assert!(!x.is_locked());
}

#[test]
fn test_remove() {
let x = PidFileLocking::lsp("lock");
assert!(x.lock().is_ok());
assert!(x.release().is_ok());
assert!(x.release().is_ok());
}

#[test]
fn test_fs_locking_stale() {
let x = PidFileLocking::lsp("stale");
assert!(x.lock().is_ok());

// lock file exists,
assert!(metadata(&x.0).is_ok());

// simulate a stale lock file
let mut x = File::create(&x.0).unwrap();
x.write_all(b"191919191919").unwrap();
x.flush().unwrap();
drop(x);

// PID=191919191919 does not exists, hopefully, and this should remove the lock file
let x = PidFileLocking::lsp("stale");
assert!(!x.is_locked());
let e = metadata(&x.0).unwrap_err().kind();
assert_eq!(e, ErrorKind::NotFound);
}
}
25 changes: 7 additions & 18 deletions forc-util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//! Utility items shared between forc crates.

use annotate_snippets::{
renderer::{AnsiColor, Style},
Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation,
Expand All @@ -26,6 +25,7 @@ use sway_types::{LineCol, SourceEngine, Span};
use sway_utils::constants;
use tracing::error;

pub mod fs_locking;
pub mod restricted;

#[macro_use]
Expand Down Expand Up @@ -156,7 +156,8 @@ pub mod tx_utils {
pub struct Salt {
/// Added salt used to derive the contract ID.
///
/// By default, this is `0x0000000000000000000000000000000000000000000000000000000000000000`.
/// By default, this is
/// `0x0000000000000000000000000000000000000000000000000000000000000000`.
#[clap(long = "salt")]
pub salt: Option<fuel_tx::Salt>,
}
Expand Down Expand Up @@ -288,7 +289,7 @@ pub fn git_checkouts_directory() -> PathBuf {
///
/// Note: This has nothing to do with `Forc.lock` files, rather this is about fd locks for
/// coordinating access to particular paths (e.g. git checkout directories).
fn fd_lock_path(path: &Path) -> PathBuf {
fn fd_lock_path<X: AsRef<Path>>(path: X) -> PathBuf {
const LOCKS_DIR_NAME: &str = ".locks";
const LOCK_EXT: &str = "forc-lock";
let file_name = hash_path(path);
Expand All @@ -298,22 +299,10 @@ fn fd_lock_path(path: &Path) -> PathBuf {
.with_extension(LOCK_EXT)
}

/// Constructs the path for the "dirty" flag file corresponding to the specified file.
///
/// This function uses a hashed representation of the original path for uniqueness.
pub fn is_dirty_path(path: &Path) -> PathBuf {
const LOCKS_DIR_NAME: &str = ".lsp-locks";
const LOCK_EXT: &str = "dirty";
let file_name = hash_path(path);
user_forc_directory()
.join(LOCKS_DIR_NAME)
.join(file_name)
.with_extension(LOCK_EXT)
}

/// Hash the path to produce a file-system friendly file name.
/// Append the file stem for improved readability.
fn hash_path(path: &Path) -> String {
fn hash_path<X: AsRef<Path>>(path: X) -> String {
let path = path.as_ref();
let mut hasher = hash_map::DefaultHasher::default();
path.hash(&mut hasher);
let hash = hasher.finish();
Expand All @@ -327,7 +316,7 @@ fn hash_path(path: &Path) -> String {
/// Create an advisory lock over the given path.
///
/// See [fd_lock_path] for details.
pub fn path_lock(path: &Path) -> Result<fd_lock::RwLock<File>> {
pub fn path_lock<X: AsRef<Path>>(path: X) -> Result<fd_lock::RwLock<File>> {
let lock_path = fd_lock_path(path);
let lock_dir = lock_path
.parent()
Expand Down
42 changes: 13 additions & 29 deletions sway-lsp/src/core/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use crate::{
error::{DirectoryError, DocumentError, LanguageServerError},
utils::document,
};
use forc_util::fs_locking::PidFileLocking;
use lsp_types::{Position, Range, TextDocumentContentChangeEvent, Url};
use ropey::Rope;
use tokio::fs::File;

#[derive(Debug, Clone)]
pub struct TextDocument {
Expand Down Expand Up @@ -111,41 +111,25 @@ impl TextDocument {
/// Marks the specified file as "dirty" by creating a corresponding flag file.
///
/// This function ensures the necessary directory structure exists before creating the flag file.
pub async fn mark_file_as_dirty(uri: &Url) -> Result<(), LanguageServerError> {
pub fn mark_file_as_dirty(uri: &Url) -> Result<(), LanguageServerError> {
let path = document::get_path_from_url(uri)?;
let dirty_file_path = forc_util::is_dirty_path(&path);
if let Some(dir) = dirty_file_path.parent() {
// Ensure the directory exists
tokio::fs::create_dir_all(dir)
.await
.map_err(|_| DirectoryError::LspLocksDirFailed)?;
}
// Create an empty "dirty" file
File::create(&dirty_file_path)
.await
.map_err(|err| DocumentError::UnableToCreateFile {
path: uri.path().to_string(),
err: err.to_string(),
})?;
Ok(())
Ok(PidFileLocking::lsp(path)
.lock()
.map_err(|_| DirectoryError::LspLocksDirFailed)?)
}

/// Removes the corresponding flag file for the specifed Url.
///
/// If the flag file does not exist, this function will do nothing.
pub async fn remove_dirty_flag(uri: &Url) -> Result<(), LanguageServerError> {
pub fn remove_dirty_flag(uri: &Url) -> Result<(), LanguageServerError> {
let path = document::get_path_from_url(uri)?;
let dirty_file_path = forc_util::is_dirty_path(&path);
if dirty_file_path.exists() {
// Remove the "dirty" file
tokio::fs::remove_file(dirty_file_path)
.await
.map_err(|err| DocumentError::UnableToRemoveFile {
path: uri.path().to_string(),
err: err.to_string(),
})?;
}
Ok(())
let uri = uri.clone();
Ok(PidFileLocking::lsp(path)
.release()
.map_err(|err| DocumentError::UnableToRemoveFile {
path: uri.path().to_string(),
err: err.to_string(),
})?)
}

#[derive(Debug)]
Expand Down
6 changes: 3 additions & 3 deletions sway-lsp/src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub async fn handle_did_change_text_document(
state: &ServerState,
params: DidChangeTextDocumentParams,
) -> Result<(), LanguageServerError> {
document::mark_file_as_dirty(&params.text_document.uri).await?;
document::mark_file_as_dirty(&params.text_document.uri)?;
let (uri, session) = state
.sessions
.uri_and_session_from_workspace(&params.text_document.uri)
Expand All @@ -100,7 +100,7 @@ pub(crate) async fn handle_did_save_text_document(
state: &ServerState,
params: DidSaveTextDocumentParams,
) -> Result<(), LanguageServerError> {
document::remove_dirty_flag(&params.text_document.uri).await?;
document::remove_dirty_flag(&params.text_document.uri)?;
let (uri, session) = state
.sessions
.uri_and_session_from_workspace(&params.text_document.uri)
Expand All @@ -124,7 +124,7 @@ pub(crate) async fn handle_did_change_watched_files(
.uri_and_session_from_workspace(&event.uri)
.await?;
if let FileChangeType::DELETED = event.typ {
document::remove_dirty_flag(&event.uri).await?;
document::remove_dirty_flag(&event.uri)?;
let _ = session.remove_document(&uri);
}
}
Expand Down
2 changes: 1 addition & 1 deletion sway-lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl LanguageServer for ServerState {
}

async fn did_close(&self, params: DidCloseTextDocumentParams) {
if let Err(err) = document::remove_dirty_flag(&params.text_document.uri).await {
if let Err(err) = document::remove_dirty_flag(&params.text_document.uri) {
tracing::error!("{}", err.to_string());
}
}
Expand Down

0 comments on commit c1c2501

Please sign in to comment.