[PATCH 3 of 6] amend: fix unlocking order - first lock then wlock

Matt Mackall mpm at selenic.com
Wed Apr 17 13:59:27 CDT 2013


On Wed, 2013-04-17 at 10:28 -0500, Kevin Bullock wrote:
> On Apr 16, 2013, at 9:11 PM, Mads Kiilerich wrote:
> 
> > # HG changeset patch
> > # User Mads Kiilerich <madski at unity3d.com>
> > # Date 1366162868 -7200
> > # Node ID 50b42260018a4739637cde911f42c04ecd402c88
> > # Parent  815cbc05cd06440e71448fd43d20142d2fc1db91
> > amend: fix unlocking order - first lock then wlock
> > 
> > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> > --- a/mercurial/cmdutil.py
> > +++ b/mercurial/cmdutil.py
> > @@ -1790,7 +1790,7 @@ def amend(ui, repo, commitfunc, old, ext
> >     finally:
> >         if newid is None:
> >             repo.dirstate.invalidate()
> > -        lockmod.release(wlock, lock)
> > +        lockmod.release(lock, wlock)
> 
> This contradicts the example code on <http://mercurial.selenic.com/wiki/LockingDesign>.

Yes, this change is incorrect. Locks should always be acquired in a
well-defined order and released in the opposite order as a matter of
good practice. Otherwise you risk deadlock.

As a practical matter, there's no risk here, since we're going to
release both locks immediately. But it's possible we might currently or
in the future rely on the rule for correctness and/or ordering in our
release callbacks.

I'll fix 'em up.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list