[PATCH 1 of 6] clone: improve locking pattern for hg clone

Durham Goode durham at fb.com
Thu May 18 18:23:55 UTC 2017


# HG changeset patch
# User Durham Goode <durham at fb.com>
# Date 1495129620 25200
#      Thu May 18 10:47:00 2017 -0700
# Node ID 19be454ee16925d01da8f9d329cb48556083da1b
# Parent  8a87bfc5bebbbe0ac996ac8e047a029eb931af45
clone: improve locking pattern for hg clone

Previously, in some cases hg.clone() would take the lock via unconventional
means (since the repo didn't exist yet). If something later on tried to take the
lock via normal means (repo.lock()), it would enter a permanent deadlock since
the lock was held by the process, but the repo object didn't realize it since
the lock was initially taken without using repo.lock().

The fix is to release the lock once we've boostrapped the clone and switch to a
normal repo.lock version.

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -586,6 +586,8 @@ def clone(ui, peeropts, source, dest=Non
             destpeer = peer(srcrepo, peeropts, dest)
             srcrepo.hook('outgoing', source='clone',
                           node=node.hex(node.nullid))
+            release(destlock)
+            destlock = None
         else:
             try:
                 destpeer = peer(srcrepo or ui, peeropts, dest, create=True)
@@ -628,52 +630,58 @@ def clone(ui, peeropts, source, dest=Non
 
         destrepo = destpeer.local()
         if destrepo:
-            template = uimod.samplehgrcs['cloned']
-            fp = destrepo.vfs("hgrc", "w", text=True)
-            u = util.url(abspath)
-            u.passwd = None
-            defaulturl = str(u)
-            fp.write(template % defaulturl)
-            fp.close()
+            wlock = lock = None
+            try:
+                wlock = destrepo.wlock()
+                lock = destrepo.lock()
+                template = uimod.samplehgrcs['cloned']
+                fp = destrepo.vfs("hgrc", "w", text=True)
+                u = util.url(abspath)
+                u.passwd = None
+                defaulturl = str(u)
+                fp.write(template % defaulturl)
+                fp.close()
 
-            destrepo.ui.setconfig('paths', 'default', defaulturl, 'clone')
+                destrepo.ui.setconfig('paths', 'default', defaulturl, 'clone')
 
-            if update:
-                if update is not True:
-                    checkout = srcpeer.lookup(update)
-                uprev = None
-                status = None
-                if checkout is not None:
-                    try:
-                        uprev = destrepo.lookup(checkout)
-                    except error.RepoLookupError:
-                        if update is not True:
+                if update:
+                    if update is not True:
+                        checkout = srcpeer.lookup(update)
+                    uprev = None
+                    status = None
+                    if checkout is not None:
+                        try:
+                            uprev = destrepo.lookup(checkout)
+                        except error.RepoLookupError:
+                            if update is not True:
+                                try:
+                                    uprev = destrepo.lookup(update)
+                                except error.RepoLookupError:
+                                    pass
+                    if uprev is None:
+                        try:
+                            uprev = destrepo._bookmarks['@']
+                            update = '@'
+                            bn = destrepo[uprev].branch()
+                            if bn == 'default':
+                                status = _("updating to bookmark @\n")
+                            else:
+                                status = (_("updating to bookmark @ on "
+                                            "branch %s\n") % bn)
+                        except KeyError:
                             try:
-                                uprev = destrepo.lookup(update)
+                                uprev = destrepo.branchtip('default')
                             except error.RepoLookupError:
-                                pass
-                if uprev is None:
-                    try:
-                        uprev = destrepo._bookmarks['@']
-                        update = '@'
+                                uprev = destrepo.lookup('tip')
+                    if not status:
                         bn = destrepo[uprev].branch()
-                        if bn == 'default':
-                            status = _("updating to bookmark @\n")
-                        else:
-                            status = (_("updating to bookmark @ on branch %s\n")
-                                      % bn)
-                    except KeyError:
-                        try:
-                            uprev = destrepo.branchtip('default')
-                        except error.RepoLookupError:
-                            uprev = destrepo.lookup('tip')
-                if not status:
-                    bn = destrepo[uprev].branch()
-                    status = _("updating to branch %s\n") % bn
-                destrepo.ui.status(status)
-                _update(destrepo, uprev)
-                if update in destrepo._bookmarks:
-                    bookmarks.activate(destrepo, update)
+                        status = _("updating to branch %s\n") % bn
+                    destrepo.ui.status(status)
+                    _update(destrepo, uprev)
+                    if update in destrepo._bookmarks:
+                        bookmarks.activate(destrepo, update)
+            finally:
+                release(lock, wlock)
     finally:
         release(srclock, destlock)
         if cleandir is not None:


More information about the Mercurial-devel mailing list