##// END OF EJS Templates
lock: fix race in lock-breaking code...
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

File last commit:

r43376:d783f945 default
r44108:039fbd14 default
Show More
logexchange.py
163 lines | 4.9 KiB | text/x-python | PythonLexer
# logexchange.py
#
# Copyright 2017 Augie Fackler <raf@durin42.com>
# Copyright 2017 Sean Farley <sean@farley.io>
#
# This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version.
from __future__ import absolute_import
from .node import hex
from . import (
pycompat,
util,
vfs as vfsmod,
)
# directory name in .hg/ in which remotenames files will be present
remotenamedir = b'logexchange'
def readremotenamefile(repo, filename):
"""
reads a file from .hg/logexchange/ directory and yields it's content
filename: the file to be read
yield a tuple (node, remotepath, name)
"""
vfs = vfsmod.vfs(repo.vfs.join(remotenamedir))
if not vfs.exists(filename):
return
f = vfs(filename)
lineno = 0
for line in f:
line = line.strip()
if not line:
continue
# contains the version number
if lineno == 0:
lineno += 1
try:
node, remote, rname = line.split(b'\0')
yield node, remote, rname
except ValueError:
pass
f.close()
def readremotenames(repo):
"""
read the details about the remotenames stored in .hg/logexchange/ and
yields a tuple (node, remotepath, name). It does not yields information
about whether an entry yielded is branch or bookmark. To get that
information, call the respective functions.
"""
for bmentry in readremotenamefile(repo, b'bookmarks'):
yield bmentry
for branchentry in readremotenamefile(repo, b'branches'):
yield branchentry
def writeremotenamefile(repo, remotepath, names, nametype):
vfs = vfsmod.vfs(repo.vfs.join(remotenamedir))
f = vfs(nametype, b'w', atomictemp=True)
# write the storage version info on top of file
# version '0' represents the very initial version of the storage format
f.write(b'0\n\n')
olddata = set(readremotenamefile(repo, nametype))
# re-save the data from a different remote than this one.
for node, oldpath, rname in sorted(olddata):
if oldpath != remotepath:
f.write(b'%s\0%s\0%s\n' % (node, oldpath, rname))
for name, node in sorted(pycompat.iteritems(names)):
if nametype == b"branches":
for n in node:
f.write(b'%s\0%s\0%s\n' % (n, remotepath, name))
elif nametype == b"bookmarks":
if node:
f.write(b'%s\0%s\0%s\n' % (node, remotepath, name))
f.close()
def saveremotenames(repo, remotepath, branches=None, bookmarks=None):
"""
save remotenames i.e. remotebookmarks and remotebranches in their
respective files under ".hg/logexchange/" directory.
"""
wlock = repo.wlock()
try:
if bookmarks:
writeremotenamefile(repo, remotepath, bookmarks, b'bookmarks')
if branches:
writeremotenamefile(repo, remotepath, branches, b'branches')
finally:
wlock.release()
def activepath(repo, remote):
"""returns remote path"""
# is the remote a local peer
local = remote.local()
# determine the remote path from the repo, if possible; else just
# use the string given to us
rpath = remote
if local:
rpath = util.pconvert(remote._repo.root)
elif not isinstance(remote, bytes):
rpath = remote._url
# represent the remotepath with user defined path name if exists
for path, url in repo.ui.configitems(b'paths'):
# remove auth info from user defined url
noauthurl = util.removeauth(url)
# Standardize on unix style paths, otherwise some {remotenames} end up
# being an absolute path on Windows.
url = util.pconvert(bytes(url))
noauthurl = util.pconvert(noauthurl)
if url == rpath or noauthurl == rpath:
rpath = path
break
return rpath
def pullremotenames(localrepo, remoterepo):
"""
pulls bookmarks and branches information of the remote repo during a
pull or clone operation.
localrepo is our local repository
remoterepo is the peer instance
"""
remotepath = activepath(localrepo, remoterepo)
with remoterepo.commandexecutor() as e:
bookmarks = e.callcommand(
b'listkeys', {b'namespace': b'bookmarks',}
).result()
# on a push, we don't want to keep obsolete heads since
# they won't show up as heads on the next pull, so we
# remove them here otherwise we would require the user
# to issue a pull to refresh the storage
bmap = {}
repo = localrepo.unfiltered()
with remoterepo.commandexecutor() as e:
branchmap = e.callcommand(b'branchmap', {}).result()
for branch, nodes in pycompat.iteritems(branchmap):
bmap[branch] = []
for node in nodes:
if node in repo and not repo[node].obsolete():
bmap[branch].append(hex(node))
saveremotenames(localrepo, remotepath, bmap, bookmarks)