[PATCH 1 of 2 RFC] lock: support flock
Jun Wu
quark at fb.com
Fri May 19 16:33:11 UTC 2017
# HG changeset patch
# User Jun Wu <quark at fb.com>
# Date 1495048786 25200
# Wed May 17 12:19:46 2017 -0700
# Node ID d1ddcac58ac7085b1b0238b74e38871343730c91
# Parent 763d7292569138886c3fdf179f7e688351bfb212
# Available At https://bitbucket.org/quark-zju/hg-draft
# hg pull https://bitbucket.org/quark-zju/hg-draft -r d1ddcac58ac7
lock: support flock
This patch makes it possible to use flock internally.
flock is more reliable in Linux's pid namespace use-case than file-existence
test, because it works without a /proc filesystem and does not have deadlock
issue if an hg process is killed.
diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -48,5 +48,10 @@ class lock(object):
Typically used via localrepository.lock() to lock the repository
store (.hg/store/) or localrepository.wlock() to lock everything
- else under .hg/.'''
+ else under .hg/.
+
+ If flockpath is not None, flock will be used instead of file-existence
+ lock. The lock file will still be used to provide lock information but it
+ is not effective for locking purpose.
+ '''
# lock is symlink on platforms that support it, file on others.
@@ -61,5 +66,6 @@ class lock(object):
def __init__(self, vfs, file, timeout=-1, releasefn=None, acquirefn=None,
- desc=None, inheritchecker=None, parentlock=None):
+ desc=None, inheritchecker=None, parentlock=None,
+ flockpath=None):
self.vfs = vfs
self.f = file
@@ -72,4 +78,6 @@ class lock(object):
self.parentlock = parentlock
self._parentheld = False
+ self._flockpath = flockpath
+ self._flockfd = None
self._inherited = False
self.postrelease = []
@@ -97,4 +105,11 @@ class lock(object):
self.release()
+ def _getflockfd(self):
+ if self._flockfd is None:
+ if self._flockpath is not None:
+ self._flockfd = os.open(self.vfs.join(self._flockpath),
+ os.O_RDONLY)
+ return self._flockfd
+
def _getpid(self):
# wrapper around util.getpid() to make testing easier
@@ -127,9 +142,14 @@ class lock(object):
retry -= 1
try:
- self.vfs.makelock(lockname, self.f)
+ self.vfs.makelock(lockname, self.f, self._getflockfd())
self.held = 1
except (OSError, IOError) as why:
- if why.errno == errno.EEXIST:
+ # EAGAIN: flock fails, EEXIST: non-flock fails
+ if why.errno in (errno.EEXIST, errno.EAGAIN):
locker = self._readlock()
+ # for flock case, locker may be outdated but surely some
+ # process is alive and taking the lock
+ if why.errno == errno.EAGAIN and locker is None:
+ locker = 'unknown'
if locker is None:
continue
@@ -163,4 +183,8 @@ class lock(object):
Returns None if no lock exists, pid for old-style locks, and host:pid
for new-style locks.
+
+ Note: in flock's case, _readlock() is a best-effort and could be
+ non-reliable after a makelock failure. It's still reliable if makelock
+ succeeded.
"""
try:
@@ -172,4 +196,8 @@ class lock(object):
def _testlock(self, locker):
+ if self._flockpath is not None:
+ # when flock is used, locker couldn't be dead (if it were, the OS
+ # will release its flock and we should not hit this code path)
+ return locker
if locker is None:
return None
@@ -260,4 +288,8 @@ class lock(object):
except OSError:
pass
+ # release flock
+ if self._flockfd is not None:
+ os.close(self._flockfd)
+ self._flockfd = None
# The postrelease functions typically assume the lock is not held
# at all.
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1080,4 +1080,7 @@ def checksignature(func):
}
+# a whitelist of known filesystems where flock works reliably
+_flockfswhitelist = _hardlinkfswhitelist
+
def copyfile(src, dest, hardlink=False, copystat=False, checkambig=False):
'''copy a file, preserving mode and optionally other stat info like
@@ -1233,16 +1236,40 @@ if safehasattr(time, "perf_counter"):
timer = time.perf_counter
-def makelock(info, pathname):
+def makelock(info, pathname, flockfd=None):
+ openflags = os.O_CREAT | os.O_WRONLY
+ # if flockfd is provided, use it as the source of truth of locking. flockfd
+ # should be on a same filesystem of pathname so the sanity check about
+ # filesystem type works.
+ if flockfd is not None:
+ fstype = getfstype(os.path.dirname(pathname)) or _('(unknown)')
+ if fstype not in _flockfswhitelist:
+ raise error.Abort(_('filesystem %s is not allowed to use flock')
+ % fstype)
+ import fcntl # not available on all platforms / code paths
+ fcntl.flock(flockfd, fcntl.LOCK_NB | fcntl.LOCK_EX) # may raise EAGAIN
+ symname = '%s.tmp%d' % (pathname, os.getpid())
+ else:
+ openflags |= os.O_EXCL
+ symname = pathname
+
try:
- return os.symlink(info, pathname)
- except OSError as why:
- if why.errno == errno.EEXIST:
- raise
- except AttributeError: # no symlink in os
- pass
-
- ld = os.open(pathname, os.O_CREAT | os.O_WRONLY | os.O_EXCL)
- os.write(ld, info)
- os.close(ld)
+ try:
+ os.symlink(info, symname)
+ if symname != pathname:
+ os.rename(symname, pathname)
+ return
+ except OSError as why:
+ if why.errno == errno.EEXIST:
+ raise
+ except AttributeError: # no symlink in os
+ pass
+
+ ld = os.open(pathname, openflags)
+ os.write(ld, info)
+ os.close(ld)
+ except Exception:
+ if flockfd is not None:
+ fcntl.flock(flockfd, fcntl.LOCK_UN)
+ raise
def readlock(pathname):
diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -147,6 +147,6 @@ class abstractvfs(object):
return util.makedirs(self.join(path), mode)
- def makelock(self, info, path):
- return util.makelock(info, self.join(path))
+ def makelock(self, info, path, flockfd=None):
+ return util.makelock(info, self.join(path), flockfd)
def mkdir(self, path=None):
diff --git a/tests/test-lock.py b/tests/test-lock.py
--- a/tests/test-lock.py
+++ b/tests/test-lock.py
@@ -33,5 +33,5 @@ class lockwrapper(lock.lock):
class teststate(object):
- def __init__(self, testcase, dir, pidoffset=0):
+ def __init__(self, testcase, dir, pidoffset=0, useflock=False):
self._testcase = testcase
self._acquirecalled = False
@@ -40,6 +40,9 @@ class teststate(object):
self.vfs = vfsmod.vfs(dir, audit=False)
self._pidoffset = pidoffset
+ self._useflock = useflock
def makelock(self, *args, **kwargs):
+ if self._useflock:
+ kwargs['flockpath'] = self.vfs.base
l = lockwrapper(self._pidoffset, self.vfs, testlockname,
releasefn=self.releasefn, acquirefn=self.acquirefn,
@@ -106,6 +109,9 @@ class teststate(object):
class testlock(unittest.TestCase):
+ useflock = False
+
def testlock(self):
- state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()))
+ state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()),
+ useflock=self.useflock)
lock = state.makelock()
state.assertacquirecalled(True)
@@ -116,5 +122,6 @@ class testlock(unittest.TestCase):
def testrecursivelock(self):
- state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()))
+ state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()),
+ useflock=self.useflock)
lock = state.makelock()
state.assertacquirecalled(True)
@@ -136,5 +143,6 @@ class testlock(unittest.TestCase):
def testlockfork(self):
- state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()))
+ state = teststate(self, tempfile.mkdtemp(dir=os.getcwd()),
+ useflock=self.useflock)
lock = state.makelock()
state.assertacquirecalled(True)
@@ -156,5 +164,5 @@ class testlock(unittest.TestCase):
def testinheritlock(self):
d = tempfile.mkdtemp(dir=os.getcwd())
- parentstate = teststate(self, d)
+ parentstate = teststate(self, d, useflock=self.useflock)
parentlock = parentstate.makelock()
parentstate.assertacquirecalled(True)
@@ -166,5 +174,5 @@ class testlock(unittest.TestCase):
parentstate.assertlockexists(True)
- childstate = teststate(self, d, pidoffset=1)
+ childstate = teststate(self, d, pidoffset=1, useflock=self.useflock)
childlock = childstate.makelock(parentlock=lockname)
childstate.assertacquirecalled(True)
@@ -186,5 +194,5 @@ class testlock(unittest.TestCase):
def testmultilock(self):
d = tempfile.mkdtemp(dir=os.getcwd())
- state0 = teststate(self, d)
+ state0 = teststate(self, d, useflock=self.useflock)
lock0 = state0.makelock()
state0.assertacquirecalled(True)
@@ -195,5 +203,5 @@ class testlock(unittest.TestCase):
state0.assertlockexists(True)
- state1 = teststate(self, d, pidoffset=1)
+ state1 = teststate(self, d, pidoffset=1, useflock=self.useflock)
lock1 = state1.makelock(parentlock=lock0name)
state1.assertacquirecalled(True)
@@ -205,5 +213,5 @@ class testlock(unittest.TestCase):
self.assertEqual(lock0name, lock1name)
- state2 = teststate(self, d, pidoffset=2)
+ state2 = teststate(self, d, pidoffset=2, useflock=self.useflock)
lock2 = state2.makelock(parentlock=lock1name)
state2.assertacquirecalled(True)
@@ -227,5 +235,5 @@ class testlock(unittest.TestCase):
def testinheritlockfork(self):
d = tempfile.mkdtemp(dir=os.getcwd())
- parentstate = teststate(self, d)
+ parentstate = teststate(self, d, useflock=self.useflock)
parentlock = parentstate.makelock()
parentstate.assertacquirecalled(True)
@@ -233,5 +241,5 @@ class testlock(unittest.TestCase):
# set up lock inheritance
with parentlock.inherit() as lockname:
- childstate = teststate(self, d, pidoffset=1)
+ childstate = teststate(self, d, pidoffset=1, useflock=self.useflock)
childlock = childstate.makelock(parentlock=lockname)
childstate.assertacquirecalled(True)
@@ -255,5 +263,5 @@ class testlock(unittest.TestCase):
def testinheritcheck(self):
d = tempfile.mkdtemp(dir=os.getcwd())
- state = teststate(self, d)
+ state = teststate(self, d, useflock=self.useflock)
def check():
raise error.LockInheritanceContractViolation('check failed')
@@ -275,5 +283,5 @@ class testlock(unittest.TestCase):
d = tempfile.mkdtemp(dir=os.getcwd())
- state = teststate(self, d)
+ state = teststate(self, d, useflock=self.useflock)
def emulatefrequentlock(*args):
@@ -293,4 +301,7 @@ class testlock(unittest.TestCase):
state.assertlockexists(False)
+class testflock(testlock):
+ useflock = True
+
if __name__ == '__main__':
silenttestrunner.main(__name__)
More information about the Mercurial-devel
mailing list