##// 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:

r43347:687b865b default
r44108:039fbd14 default
Show More
rewriteutil.py
55 lines | 1.6 KiB | text/x-python | PythonLexer
Pulkit Goyal
rewriteutil: add utility function to check if we can create new unstable cset...
r35242 # rewriteutil.py - utility functions for rewriting changesets
#
# Copyright 2017 Octobus <contact@octobus.net>
#
# 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
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 from .i18n import _
Pulkit Goyal
rewriteutil: add utility function to check if we can create new unstable cset...
r35242 from . import (
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 error,
node,
Pulkit Goyal
rewriteutil: add utility function to check if we can create new unstable cset...
r35242 obsolete,
revset,
)
Augie Fackler
style: run a patched black on a subset of mercurial...
r43345
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 def precheck(repo, revs, action=b'rewrite'):
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 """check if revs can be rewritten
action is used to control the error message.
Make sure this function is called after taking the lock.
"""
if node.nullrev in revs:
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 msg = _(b"cannot %s null changeset") % action
hint = _(b"no changeset checked out")
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 raise error.Abort(msg, hint=hint)
if len(repo[None].parents()) > 1:
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 raise error.Abort(_(b"cannot %s while merging") % action)
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 publicrevs = repo.revs(b'%ld and public()', revs)
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 if publicrevs:
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 msg = _(b"cannot %s public changesets") % action
hint = _(b"see 'hg help phases' for details")
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243 raise error.Abort(msg, hint=hint)
newunstable = disallowednewunstable(repo, revs)
if newunstable:
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 raise error.Abort(_(b"cannot %s changeset with children") % action)
Pulkit Goyal
rewriteutil: add a precheck function to check if revs can be rewritten...
r35243
Augie Fackler
style: run a patched black on a subset of mercurial...
r43345
Pulkit Goyal
rewriteutil: add utility function to check if we can create new unstable cset...
r35242 def disallowednewunstable(repo, revs):
"""Checks whether editing the revs will create new unstable changesets and
are we allowed to create them.
To allow new unstable changesets, set the config:
`experimental.evolution.allowunstable=True`
"""
allowunstable = obsolete.isenabled(repo, obsolete.allowunstableopt)
if allowunstable:
return revset.baseset()
Augie Fackler
formatting: byteify all mercurial/ and hgext/ string literals...
r43347 return repo.revs(b"(%ld::) - %ld", revs, revs)