Patchwork [2,of,2] shelve: acquire lock in the right order

login
register
mail settings
Submitter Pierre-Yves David
Date April 13, 2015, 5:28 p.m.
Message ID <7d10775076a3ca8ee7d3.1428946116@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8633/
State Accepted
Headers show

Comments

Pierre-Yves David - April 13, 2015, 5:28 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1428865171 14400
#      Sun Apr 12 14:59:31 2015 -0400
# Node ID 7d10775076a3ca8ee7d3544112a83483313905c8
# Parent  55c24d69aea7ec2690dee8ff0111112c85d8dfaa
shelve: acquire lock in the right order

Text book says that 'wlock' should be acquired before 'lock'.
Caught through developer warning.
Martin von Zweigbergk - April 13, 2015, 5:53 p.m.
On Mon, Apr 13, 2015 at 10:30 AM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1428865171 14400
> #      Sun Apr 12 14:59:31 2015 -0400
> # Node ID 7d10775076a3ca8ee7d3544112a83483313905c8
> # Parent  55c24d69aea7ec2690dee8ff0111112c85d8dfaa
> shelve: acquire lock in the right order
>
> Text book says that 'wlock' should be acquired before 'lock'.
>

Where is the text book? I looked at localrepo.(w)lock() and it didn't
mention anything. Should it?

Thanks, I'm pushing this to the clowncopter.



> Caught through developer warning.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -542,12 +542,12 @@ def unshelve(ui, repo, *shelved, **opts)
>          raise util.Abort(_("shelved change '%s' not found") % basename)
>
>      oldquiet = ui.quiet
>      wlock = lock = tr = None
>      try:
> +        wlock = repo.wlock()
>          lock = repo.lock()
> -        wlock = repo.wlock()
>
>          tr = repo.transaction('unshelve', report=lambda x: None)
>          oldtiprev = len(repo)
>
>          pctx = repo['.']
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - April 13, 2015, 5:57 p.m.
On 04/13/2015 01:53 PM, Martin von Zweigbergk wrote:
>
> On Mon, Apr 13, 2015 at 10:30 AM Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@fb.com
>     <mailto:pierre-yves.david@fb.com>>
>     # Date 1428865171 14400
>     #      Sun Apr 12 14:59:31 2015 -0400
>     # Node ID 7d10775076a3ca8ee7d3544112a83483313905c8
>     # Parent  55c24d69aea7ec2690dee8ff0111112c85d8dfaa
>     shelve: acquire lock in the right order
>
>     Text book says that 'wlock' should be acquired before 'lock'.
>
>
> Where is the text book? I looked at localrepo.(w)lock() and it didn't
> mention anything. Should it?

Somewhere http://mercurial.selenic.com/wiki/LockingDesign

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -542,12 +542,12 @@  def unshelve(ui, repo, *shelved, **opts)
         raise util.Abort(_("shelved change '%s' not found") % basename)
 
     oldquiet = ui.quiet
     wlock = lock = tr = None
     try:
+        wlock = repo.wlock()
         lock = repo.lock()
-        wlock = repo.wlock()
 
         tr = repo.transaction('unshelve', report=lambda x: None)
         oldtiprev = len(repo)
 
         pctx = repo['.']