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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 31317

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)),