view tests/helpers-testrepo.sh @ 43709:039fbd14d4e2

lock: fix race in lock-breaking code With low frequency, I see hg pulls fail with output like: abort: no such file or directory: .hg/store/lock I think what happens is, in lock.py, in: def _testlock(self, locker): if not self._lockshouldbebroken(locker): return locker # if locker dead, break lock. must do this with another lock # held, or can race and break valid lock. try: with lock(self.vfs, self.f + b'.break', timeout=0): self.vfs.unlink(self.f) except error.LockError: return locker if a lock is breakable on disk, and two hg processes concurrently get to the "if locker dead" comment, a possible interleaving is: process1 finishes executing the function and then process2 finishes executing the function. If that happens, process2 will either get ENOENT in self.vfs.unlink (resulting in the spurious failure above), or break a valid lock and potentially cause repository corruption. The fix is simple enough: make sure the lock is breakable _inside_ the critical section, because only then can we know that no other process can invalidate our knowledge on the lock on disk. I don't think there are tests for this. I've tested this manually with: diff --git a/mercurial/lock.py b/mercurial/lock.py --- a/mercurial/lock.py +++ b/mercurial/lock.py @@ -351,6 +351,8 @@ class lock(object): if not self._lockshouldbebroken(locker): return locker + import random + time.sleep(1. + random.random()) # if locker dead, break lock. must do this with another lock # held, or can race and break valid lock. try: @@ -358,6 +360,7 @@ class lock(object): self.vfs.unlink(self.f) except error.LockError: return locker + time.sleep(1) def testlock(self): """return id of locker if lock is valid, else None. and I see this change of behavior before/after this commit: $ $hg init repo $ cd repo $ ln -s $HOSTNAME/effffffc:987654321 .hg/wlock $ touch a $ $hg commit -Am_ & $hg commit -Am _; wait -abort: No such file or directory: '/tmp/repo/.hg/wlock' adding a +warning: ignoring unknown working parent 679a8959a8ca! +nothing changed Differential Revision: https://phab.mercurial-scm.org/D7199
author Valentin Gatien-Baron <valentin.gatienbaron@gmail.com>
date Mon, 18 Nov 2019 20:10:38 -0800
parents 152f1b47e0ad
children
line wrap: on
line source

# In most cases, the mercurial repository can be read by the bundled hg, but
# that isn't always true because third-party extensions may change the store
# format, for example. In which case, the system hg installation is used.
#
# We want to use the hg version being tested when interacting with the test
# repository, and the system hg when interacting with the mercurial source code
# repository.
#
# The mercurial source repository was typically orignally cloned with the
# system mercurial installation, and may require extensions or settings from
# the system installation.

if [ -n "$HGTESTEXTRAEXTENSIONS" ]; then
    for extension in $HGTESTEXTRAEXTENSIONS; do
        extraoptions="$extraoptions --config extensions.$extension=!"
    done
fi

syshg () {
    (
        syshgenv
        exec hg "$@"
    )
}

# Revert the environment so that running "hg" runs the system hg
# rather than the test hg installation.
syshgenv () {
    . "$HGTEST_RESTOREENV"
    HGPLAIN=1
    export HGPLAIN
}

# The test-repo is a live hg repository which may have evolution markers
# created, e.g. when a ~/.hgrc enabled evolution.
#
# Tests may be run using a custom HGRCPATH, which do not enable evolution
# markers by default.
#
# If test-repo includes evolution markers, and we do not enable evolution
# markers, hg will occasionally complain when it notices them, which disrupts
# tests resulting in sporadic failures.
#
# Since we aren't performing any write operations on the test-repo, there's
# no harm in telling hg that we support evolution markers, which is what the
# following lines for the hgrc file do:
cat >> "$HGRCPATH" << EOF
[experimental]
evolution = createmarkers
EOF

# Use the system hg command if the bundled hg can't read the repository with
# no warning nor error.
if [ -n "`hg id -R "$TESTDIR/.." 2>&1 >/dev/null`" ]; then
    alias testrepohg=syshg
    alias testrepohgenv=syshgenv
else
    alias testrepohg="hg $extraoptions"
    alias testrepohgenv=:
fi