From patchwork Thu May 18 18:25:51 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [1,of,6] clone: improve locking pattern for hg clone
From: Durham Goode <durham@fb.com>
X-Patchwork-Id: 20682
Message-Id: <ab20cf9b-bb8a-2a0c-8b70-bdc5cb602a07@fb.com>
To: <mercurial-devel@mercurial-scm.org>
Date: Thu, 18 May 2017 11:25:51 -0700

On 5/18/17 11:23 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@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.
>

This one is easier to review with whitespace diff off:

create=True)
@@ -628,6 +630,10 @@ def clone(ui, peeropts, source, dest=Non

          destrepo = destpeer.local()
          if destrepo:
+            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)
@@ -660,8 +666,8 @@ def clone(ui, peeropts, source, dest=Non
                          if bn == 'default':
                              status = _("updating to bookmark @\n")
                          else:
-                            status = (_("updating to bookmark @ on 
branch %s\n")
-                                      % bn)
+                                status = (_("updating to bookmark @ on "
+                                            "branch %s\n") % bn)
                      except KeyError:
                          try:
                              uprev = destrepo.branchtip('default')
@@ -675,6 +681,8 @@ def clone(ui, peeropts, source, dest=Non
                  if update in destrepo._bookmarks:
                      bookmarks.activate(destrepo, update)
      finally:
+                release(lock, wlock)
+    finally:
          release(srclock, destlock)
          if cleandir is not None:
              shutil.rmtree(cleandir, True)

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,