facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-06 14:58:03 +03:00
Code Issues Projects Releases Wiki Activity

repolock: always clean up legacy lock path

Browse Source 
Summary:
The rust repolock still creates the legacy lock files (e.g. .hg/wlock). Previously in the python_and_rust transitional lock mode, rust would try to check if the Python code was "really" using the legacy paths to avoid deleting the legacy lock files out from under Python. Now that we are in rust_only mode, this logic is not necessary, and in fact can cause the .hg/wlock file to get stuck on Windows if an interrupted operation doesn't clean up .hg/wlock for whatever reason.

On Windows, the legacy locking is achieved using the "you can't rename file into an existing file" property, so the mere presence of the file means the file is "locked". So, if the wlock file failed to get cleaned up for whatever reason, Rust would see that it already existed and leave it alone, assuming Python would clean it up. But with Python locking disabled, that means no one would clean it up.

Reviewed By: sggutier

Differential Revision: D41005455

fbshipit-source-id: a83e92208f9f9a87a528e501a5f4a31ede948c5c
This commit is contained in:
Muir Manders 2022-11-07 07:04:04 -08:00 committed by Facebook GitHub Bot
parent 7204c6fd17
commit 67070d05cc
3 changed files with 12 additions and 137 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
eden/scm/lib/repolock/src/lib.rs
View File

@ -8,7 +8,6 @@
use std::error;
use std::fmt;
use std::fs::File;
use std::fs::OpenOptions;
use std::fs::Permissions;
use std::io::Write;
use std::ops::Add;
@ -137,38 +136,15 @@ pub fn try_lock(dir: &Path, name: &str, contents: &[u8]) -> anyhow::Result<LockH
}
};
let mut legacy_already_locked = false;
if legacy_path.exists() {
if cfg!(windows) {
// If file exists, lock is active. Windows uses the
// can't-rename-into-existing-file property to implement
// locking.
legacy_already_locked = true
} else if let Ok(f) = OpenOptions::new().write(true).open(&legacy_path) {
legacy_already_locked = f.try_lock_exclusive().is_err();
}
}
// Create the legacy lock file to maintain compatibility for
// external code that checks directly for .hg/wlock as an
// indication of "is an hg operation in progress".
let mut legacy_lock = None;
if !legacy_already_locked {
if let Ok(mut legacy_file) = File::create(&legacy_path) {
// Also write lock contents for compatibility with Python readers.
let _ = legacy_file.write_all(contents.as_ref());
if let Ok(mut legacy_file) = File::create(&legacy_path) {
// Also write lock contents for compatibility with Python readers.
let _ = legacy_file.write_all(contents.as_ref());
#[cfg(unix)]
{
let _ = legacy_file.set_permissions(Permissions::from_mode(0o644));
// Take the lock so Python doesn't delete file
// when attempting to take lock. We also need to
// hold on to the file to keep the lock.
let _ = legacy_file.try_lock_exclusive();
legacy_lock = Some(legacy_file);
}
}
#[cfg(unix)]
let _ = legacy_file.set_permissions(Permissions::from_mode(0o644));
}
let mut contents_file = util::file::open(&path, "wct")?;
@ -181,20 +157,14 @@ pub fn try_lock(dir: &Path, name: &str, contents: &[u8]) -> anyhow::Result<LockH
Ok(LockHandle {
path: lock_file_path,
lock: lock_file,
legacy_path: if !legacy_already_locked {
Some(legacy_path)
} else {
None
},
legacy_lock,
legacy_path,
})
}
pub struct LockHandle {
path: PathBuf,
lock: File,
legacy_path: Option<PathBuf>,
legacy_lock: Option<File>,
legacy_path: PathBuf,
}
impl LockHandle {
@ -206,12 +176,7 @@ impl LockHandle {
}
fn unlink_legacy(&mut self) {
if let Some(path) = self.legacy_path.take() {
// Close legacy_lock file, if present.
self.legacy_lock.take();
let _ = util::path::remove_file(&path);
}
let _ = util::path::remove_file(&self.legacy_path);
}
}
@ -257,7 +222,6 @@ impl fmt::Display for LockContendedError {
#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
use std::fs;
use std::thread;
use std::thread::spawn;
@ -421,9 +385,7 @@ mod tests {
assert!(!legacy_path.exists());
}
// Legacy path does exist but isn't locked. Doesn't apply to
// Windows because mere presence of file means "locked".
#[cfg(unix)]
// Legacy path already exists - clean it up.
{
File::create(&legacy_path)?;
@ -436,24 +398,6 @@ mod tests {
assert!(!legacy_path.exists());
}
// legacy path exists and _is_ locked (this indicates python locking is also active)
{
let mut opts = fs::OpenOptions::new();
opts.create(true).write(true).truncate(true);
let legacy_file = opts.open(&legacy_path)?;
legacy_file.lock_exclusive()?;
{
let _foo_lock = try_lock(tmp.path(), "foo", "some contents".as_bytes())?;
assert!(legacy_path.exists());
}
// do not clean up legacy file
assert!(legacy_path.exists());
}
Ok(())
}
}

5
eden/scm/tests/test-lock-badness.t
View File

@ -4,6 +4,9 @@
$ configure modernclient
workingcopy.ruststatus deadlocks when calling into rust status
$ setconfig devel.lockmode=rust_only workingcopy.ruststatus=false
Prepare
$ newclientrepo a
@ -129,7 +132,6 @@ On processs waiting on another, warning after a long time (debug output on)
calling hook pre-update: hghook_pre-update.sleephalf
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cat preup-stderr
locker is still running (full unique id: '*') (glob)
waiting for lock on working directory of b held by process '*' on host * (glob)
(hint: run * to see related processes) (glob)
got lock after * seconds (glob) (?)
@ -148,7 +150,6 @@ On processs waiting on another, warning disabled, (debug output on)
calling hook pre-update: hghook_pre-update.sleephalf
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cat preup-stderr
locker is still running (full unique id: '*') (glob)
waiting for lock on working directory of b held by process '*' on host * (glob)
(hint: run * to see related processes) (glob)
got lock after * seconds (glob) (?)

70
eden/scm/tests/test-rust-lock.py
View File

@ -96,76 +96,6 @@ class testrustlock(unittest.TestCase):
self.assertNotLocked("foo")
def testpyandrust(self):
# Lock both python and rust locks.
with self.ui.configoverride({("devel", "lockmode"): "python_and_rust"}):
acquired, prereleased, postreleased = (0, 0, 0)
def acquire():
nonlocal acquired
acquired += 1
def release():
nonlocal prereleased
prereleased += 1
def postrelease():
nonlocal postreleased
postreleased += 1
with lock.trylock(
self.ui, self.vfs, "foo.lock", 5, acquirefn=acquire, releasefn=release
) as l:
l.postrelease.append(postrelease)
self.assertLocked("foo.lock")
self.assertLegacyLock("foo.lock", True)
self.assertEqual(acquired, 1)
self.assertEqual(prereleased, 0)
self.assertEqual(postreleased, 0)
self.assertNotLocked("foo.lock")
self.assertLegacyLock("foo.lock", False)
self.assertEqual(acquired, 1)
self.assertEqual(prereleased, 1)
self.assertEqual(postreleased, 1)
# Test what happens when rust lock is unavailable.
def testcontendedpyandrust(self):
with self.ui.configoverride({("devel", "lockmode"): "python_and_rust"}):
acquired, released = (0, 0)
def acquire():
nonlocal acquired
acquired += 1
def release():
nonlocal released
released += 1
with lock.rustlock(self.vfs, "foo"):
# Rust lock takes Python lock as well.
self.assertLegacyLock("foo", True)
with self.assertRaises(error.LockHeld):
lock.trylock(
self.ui,
self.vfs,
"foo",
0,
acquirefn=acquire,
releasefn=release,
)
# Rust still locked.
self.assertLocked("foo")
# Rust lock still has legacy lock.
self.assertLegacyLock("foo", True)
# Make sure we didn't call any callbacks.
self.assertEqual(acquired, 0)
self.assertEqual(released, 0)
# Make sure the devel.lockmode=rust_only flag works.
def testrustonlymode(self):
with self.ui.configoverride({("devel", "lockmode"): "rust_only"}):