[PATCH 2 of 4] devel: move the lock-checking code into core

Ryan McElroy rm at fb.com
Wed Mar 11 10:56:19 CDT 2015


On 3/10/2015 9:56 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1421405470 28800
> #      Fri Jan 16 02:51:10 2015 -0800
> # Node ID 831c69fcda436381cc27a0679c05b71d69184376
> # Parent  e9e42adc21a5c0a9bd68079bdd209b7a086ad046
> devel: move the lock-checking code into core
>
> If the developer warning are enabled, bad locking order will be reported without
> the need for the contrib extensions.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1144,10 +1144,18 @@ class localrepository(object):
>   
>       def wlock(self, wait=True):
>           '''Lock the non-store parts of the repository (everything under
>           .hg except .hg/store) and return a weak reference to the lock.
>           Use this before modifying files in .hg.'''
> +        if self.ui.develflag('check-locks'):
> +            l = self._lockref and self._lockref()
> +            if l is not None and l.held:
> +                msg = '"lock" taken before "wlock"\n'
> +                if self.ui.tracebackflag:
> +                    util.debugstacktrace(msg, 2)
> +                else:
> +                    self.ui.write_err(msg)

Same feedback as on #4: 2 is a magic number, msg is constructed before 
it's needed and not always used.

>           l = self._wlockref and self._wlockref()
>           if l is not None and l.held:
>               l.lock()
>               return l
>   
> diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-devel-warnings.t
> @@ -0,0 +1,35 @@
> +
> +  $ cat << EOF > buggylocking.py
> +  > """A small extension to acquires lock in the wrong order
> +  > """
> +  >
> +  > from mercurial import cmdutil
> +  >
> +  > cmdtable = {}
> +  > command = cmdutil.command(cmdtable)
> +  >
> +  > @command('buggylocking', [], '')
> +  > def buggylocking(ui, repo):
> +  >     lo = repo.lock()
> +  >     wl = repo.wlock()
> +  > EOF
> +
> +  $ cat << EOF >> $HGRCPATH
> +  > [extensions]
> +  > buggylocking=$TESTTMP/buggylocking.py
> +  > [devel]
> +  > all=1
> +  > EOF
> +
> +  $ hg init lock-checker
> +  $ cd lock-checker
> +  $ hg buggylocking
> +  "lock" taken before "wlock"
> +  $ cat << EOF >> $HGRCPATH
> +  > [devel]
> +  > all=0
> +  > check-locks=1
> +  > EOF
> +  $ hg buggylocking
> +  "lock" taken before "wlock"
> +  $ cd ..

Overall, I like this patchset; prefer it with Yuya's and my suggestions.


More information about the Mercurial-devel mailing list