This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11835

rhg: Initial repository locking
ClosedPublic

Authored by SimonSapin on Dec 2 2021, 11:48 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG5734b03ecf3e: rhg: Initial repository locking
Summary

Initial Rust implementation of locking based on the .hg/wlock symlink (or file),
with lock breaking when the recorded pid and hostname show that a lock was
left by a local process that is not running anymore (as it might have been
killed).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Dec 2 2021, 11:48 AM
Alphare requested changes to this revision.Dec 3 2021, 6:13 AM
Alphare added a subscriber: Alphare.

Small change, but it is different behavior, so I'll let you change that.

Otherwise the change looks good and I see no real different with the Python implementation.

rust/hg-core/src/lock.rs
126

There is not I'm afraid.

170

You can catch ENOTDIR with error.raw_os_error() == Some(20)

This revision now requires changes to proceed.Dec 3 2021, 6:13 AM
SimonSapin added inline comments.Dec 6 2021, 4:21 AM
rust/hg-core/src/lock.rs
126

Yeah it just feels weird to go through std::io::Error. I wish the libc crate exposed something to access errno, even though it might need to be more than a simple FFI binding. I’ll remove this comment.

170

Good point, I’ll change the whole match to look at errno values

SimonSapin updated this revision to Diff 31317.Dec 6 2021, 5:02 AM
Alphare accepted this revision.Dec 6 2021, 5:17 AM
This revision is now accepted and ready to land.Dec 6 2021, 5:17 AM
SimonSapin added a commit: rHG5734b03ecf3e: rhg: Initial repository locking.Dec 7 2021, 10:05 AM
Closed by commit rHG5734b03ecf3e: rhg: Initial repository locking (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 31350

rust/hg-core/src/lib.rs

}; };
pub mod copy_tracing; pub mod copy_tracing;
mod filepatterns; mod filepatterns;
pub mod matchers; pub mod matchers;
pub mod repo; pub mod repo;
pub mod revlog; pub mod revlog;
pub use revlog::*; pub use revlog::*;
pub mod config; pub mod config;
pub mod lock;
pub mod logging; pub mod logging;
pub mod operations; pub mod operations;
pub mod revset; pub mod revset;
pub mod utils; pub mod utils;
pub mod vfs; pub mod vfs;
use crate::utils::hg_path::{HgPathBuf, HgPathError}; use crate::utils::hg_path::{HgPathBuf, HgPathError};
pub use filepatterns::{ pub use filepatterns::{

rust/hg-core/src/lock.rs

  • This file was added.
//! Filesystem-based locks for local repositories
use crate::errors::HgError;
use crate::errors::HgResultExt;
use crate::utils::StrExt;
use crate::vfs::Vfs;
use std::io;
use std::io::ErrorKind;
#[derive(derive_more::From)]
pub enum LockError {
AlreadyHeld,
#[from]
Other(HgError),
}
/// Try to call `f` with the lock acquired, without waiting.
///
/// If the lock is aready held, `f` is not called and `LockError::AlreadyHeld`
/// is returned. `LockError::Io` is returned for any unexpected I/O error
/// accessing the lock file, including for removing it after `f` was called.
/// The return value of `f` is dropped in that case. If all is successful, the
/// return value of `f` is forwarded.
pub fn try_with_lock_no_wait<R>(
hg_vfs: Vfs,
lock_filename: &str,
f: impl FnOnce() -> R,
) -> Result<R, LockError> {
let our_lock_data = &*OUR_LOCK_DATA;
for _retry in 0..5 {
match make_lock(hg_vfs, lock_filename, our_lock_data) {
Ok(()) => {
let result = f();
unlock(hg_vfs, lock_filename)?;
return Ok(result);
}
Err(HgError::IoError { error, .. })
if error.kind() == ErrorKind::AlreadyExists =>
{
let lock_data = read_lock(hg_vfs, lock_filename)?;
if lock_data.is_none() {
// Lock was apparently just released, retry acquiring it
continue;
}
if !lock_should_be_broken(&lock_data) {
return Err(LockError::AlreadyHeld);
}
// The lock file is left over from a process not running
// anymore. Break it, but with another lock to
// avoid a race.
break_lock(hg_vfs, lock_filename)?;
// Retry acquiring
}
Err(error) => Err(error)?,
}
}
Err(LockError::AlreadyHeld)
}
fn break_lock(hg_vfs: Vfs, lock_filename: &str) -> Result<(), LockError> {
try_with_lock_no_wait(hg_vfs, &format!("{}.break", lock_filename), || {
// Check again in case some other process broke and
// acquired the lock in the meantime
let lock_data = read_lock(hg_vfs, lock_filename)?;
if !lock_should_be_broken(&lock_data) {
return Err(LockError::AlreadyHeld);
}
Ok(hg_vfs.remove_file(lock_filename)?)
})?
}
#[cfg(unix)]
fn make_lock(
hg_vfs: Vfs,
lock_filename: &str,
data: &str,
) -> Result<(), HgError> {
// Use a symbolic link because creating it is atomic.
// The link’s "target" contains data not representing any path.
let fake_symlink_target = data;
hg_vfs.create_symlink(lock_filename, fake_symlink_target)
}
fn read_lock(
hg_vfs: Vfs,
lock_filename: &str,
) -> Result<Option<String>, HgError> {
let link_target =
hg_vfs.read_link(lock_filename).io_not_found_as_none()?;
if let Some(target) = link_target {
let data = target
.into_os_string()
.into_string()
.map_err(|_| HgError::corrupted("non-UTF-8 lock data"))?;
Ok(Some(data))
} else {
Ok(None)
}
}
fn unlock(hg_vfs: Vfs, lock_filename: &str) -> Result<(), HgError> {
hg_vfs.remove_file(lock_filename)
}
/// Return whether the process that is/was holding the lock is known not to be
/// running anymore.
fn lock_should_be_broken(data: &Option<String>) -> bool {
(|| -> Option<bool> {
let (prefix, pid) = data.as_ref()?.split_2(':')?;
if prefix != &*LOCK_PREFIX {
return Some(false);
}
let process_is_running;
#[cfg(unix)]
{
let pid: libc::pid_t = pid.parse().ok()?;
unsafe {
let signal = 0; // Test if we could send a signal, without sending
let result = libc::kill(pid, signal);
if result == 0 {
process_is_running = true
} else {
let errno =
io::Error::last_os_error().raw_os_error().unwrap();
AlphareUnsubmitted
Not Done

There is not I'm afraid.

Alphare: There is not I'm afraid.
SimonSapinAuthorUnsubmitted
Done

Yeah it just feels weird to go through std::io::Error. I wish the libc crate exposed something to access errno, even though it might need to be more than a simple FFI binding. I’ll remove this comment.

SimonSapin: Yeah it just feels weird to go through `std::io::Error`. I wish the libc crate exposed…
process_is_running = errno != libc::ESRCH
}
}
}
Some(!process_is_running)
})()
.unwrap_or(false)
}
lazy_static::lazy_static! {
/// A string which is used to differentiate pid namespaces
///
/// It's useful to detect "dead" processes and remove stale locks with
/// confidence. Typically it's just hostname. On modern linux, we include an
/// extra Linux-specific pid namespace identifier.
static ref LOCK_PREFIX: String = {
// Note: this must match the behavior of `_getlockprefix` in `mercurial/lock.py`
/// Same as https://github.com/python/cpython/blob/v3.10.0/Modules/socketmodule.c#L5414
const BUFFER_SIZE: usize = 1024;
let mut buffer = [0_i8; BUFFER_SIZE];
let hostname_bytes = unsafe {
let result = libc::gethostname(buffer.as_mut_ptr(), BUFFER_SIZE);
if result != 0 {
panic!("gethostname: {}", io::Error::last_os_error())
}
std::ffi::CStr::from_ptr(buffer.as_mut_ptr()).to_bytes()
};
let hostname =
std::str::from_utf8(hostname_bytes).expect("non-UTF-8 hostname");
#[cfg(target_os = "linux")]
{
use std::os::linux::fs::MetadataExt;
match std::fs::metadata("/proc/self/ns/pid") {
Ok(meta) => {
return format!("{}/{:x}", hostname, meta.st_ino())
}
Err(error) => {
// TODO: match on `error.kind()` when `NotADirectory`
// is available on all supported Rust versions:
// https://github.com/rust-lang/rust/issues/86442
use libc::{
AlphareUnsubmitted
Not Done

You can catch ENOTDIR with error.raw_os_error() == Some(20)

Alphare: You can catch `ENOTDIR` with `error.raw_os_error() == Some(20)`
SimonSapinAuthorUnsubmitted
Done

Good point, I’ll change the whole match to look at errno values

SimonSapin: Good point, I’ll change the whole `match` to look at errno values
ENOENT, // ErrorKind::NotFound
ENOTDIR, // ErrorKind::NotADirectory
EACCES, // ErrorKind::PermissionDenied
};
match error.raw_os_error() {
Some(ENOENT) | Some(ENOTDIR) | Some(EACCES) => {}
_ => panic!("stat /proc/self/ns/pid: {}", error),
}
}
}
}
hostname.to_owned()
};
static ref OUR_LOCK_DATA: String = format!("{}:{}", &*LOCK_PREFIX, std::process::id());
}

rust/hg-core/src/repo.rs

use crate::changelog::Changelog; use crate::changelog::Changelog;
use crate::config::{Config, ConfigError, ConfigParseError}; use crate::config::{Config, ConfigError, ConfigParseError};
use crate::dirstate::DirstateParents; use crate::dirstate::DirstateParents;
use crate::dirstate_tree::dirstate_map::DirstateMap; use crate::dirstate_tree::dirstate_map::DirstateMap;
use crate::dirstate_tree::owning::OwningDirstateMap; use crate::dirstate_tree::owning::OwningDirstateMap;
use crate::errors::HgError; use crate::errors::HgError;
use crate::errors::HgResultExt; use crate::errors::HgResultExt;
use crate::exit_codes; use crate::exit_codes;
use crate::lock::{try_with_lock_no_wait, LockError};
use crate::manifest::{Manifest, Manifestlog}; use crate::manifest::{Manifest, Manifestlog};
use crate::revlog::filelog::Filelog; use crate::revlog::filelog::Filelog;
use crate::revlog::revlog::RevlogError; use crate::revlog::revlog::RevlogError;
use crate::utils::files::get_path_from_bytes; use crate::utils::files::get_path_from_bytes;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt; use crate::utils::SliceExt;
use crate::vfs::{is_dir, is_file, Vfs}; use crate::vfs::{is_dir, is_file, Vfs};
use crate::{requirements, NodePrefix}; use crate::{requirements, NodePrefix};
/// For accessing the working copy /// For accessing the working copy
pub fn working_directory_vfs(&self) -> Vfs<'_> { pub fn working_directory_vfs(&self) -> Vfs<'_> {
Vfs { Vfs {
base: &self.working_directory, base: &self.working_directory,
} }
} }
pub fn try_with_wlock_no_wait<R>(
&self,
f: impl FnOnce() -> R,
) -> Result<R, LockError> {
try_with_lock_no_wait(self.hg_vfs(), "wlock", f)
}
pub fn has_dirstate_v2(&self) -> bool { pub fn has_dirstate_v2(&self) -> bool {
self.requirements self.requirements
.contains(requirements::DIRSTATE_V2_REQUIREMENT) .contains(requirements::DIRSTATE_V2_REQUIREMENT)
} }
pub fn has_sparse(&self) -> bool { pub fn has_sparse(&self) -> bool {
self.requirements.contains(requirements::SPARSE_REQUIREMENT) self.requirements.contains(requirements::SPARSE_REQUIREMENT)
} }

rust/hg-core/src/utils.rs

if let Some(pos) = find_slice_in_slice(self, separator) { if let Some(pos) = find_slice_in_slice(self, separator) {
Some((&self[..pos], &self[pos + separator.len()..])) Some((&self[..pos], &self[pos + separator.len()..]))
} else { } else {
None None
} }
} }
} }
pub trait StrExt {
// TODO: Use https://doc.rust-lang.org/nightly/std/primitive.str.html#method.split_once
// once we require Rust 1.52+
fn split_2(&self, separator: char) -> Option<(&str, &str)>;
}
impl StrExt for str {
fn split_2(&self, separator: char) -> Option<(&str, &str)> {
let mut iter = self.splitn(2, separator);
let a = iter.next()?;
let b = iter.next()?;
Some((a, b))
}
}
pub trait Escaped { pub trait Escaped {
/// Return bytes escaped for display to the user /// Return bytes escaped for display to the user
fn escaped_bytes(&self) -> Vec<u8>; fn escaped_bytes(&self) -> Vec<u8>;
} }
impl Escaped for u8 { impl Escaped for u8 {
fn escaped_bytes(&self) -> Vec<u8> { fn escaped_bytes(&self) -> Vec<u8> {
let mut acc = vec![]; let mut acc = vec![];

rust/hg-core/src/vfs.rs

relative_from: impl AsRef<Path>, relative_from: impl AsRef<Path>,
relative_to: impl AsRef<Path>, relative_to: impl AsRef<Path>,
) -> Result<(), HgError> { ) -> Result<(), HgError> {
let from = self.join(relative_from); let from = self.join(relative_from);
let to = self.join(relative_to); let to = self.join(relative_to);
std::fs::rename(&from, &to) std::fs::rename(&from, &to)
.with_context(|| IoErrorContext::RenamingFile { from, to }) .with_context(|| IoErrorContext::RenamingFile { from, to })
} }
pub fn remove_file(
&self,
relative_path: impl AsRef<Path>,
) -> Result<(), HgError> {
let path = self.join(relative_path);
std::fs::remove_file(&path)
.with_context(|| IoErrorContext::RemovingFile(path))
}
#[cfg(unix)]
pub fn create_symlink(
&self,
relative_link_path: impl AsRef<Path>,
target_path: impl AsRef<Path>,
) -> Result<(), HgError> {
let link_path = self.join(relative_link_path);
std::os::unix::fs::symlink(target_path, &link_path)
.with_context(|| IoErrorContext::WritingFile(link_path))
}
} }
fn fs_metadata( fn fs_metadata(
path: impl AsRef<Path>, path: impl AsRef<Path>,
) -> Result<Option<std::fs::Metadata>, HgError> { ) -> Result<Option<std::fs::Metadata>, HgError> {
let path = path.as_ref(); let path = path.as_ref();
match std::fs::metadata(path) { match std::fs::metadata(path) {
Ok(meta) => Ok(Some(meta)), Ok(meta) => Ok(Some(meta)),