hg: obtain lock when creating share from pooled repo (issue5104) stable 3.7.2
authorGregory Szorc <gregory.szorc@gmail.com>
Sat, 27 Feb 2016 18:22:49 -0800
branchstable
changeset 28289 d493d64757eb
parent 28288 e417e4512b0f
child 28290 9a7c27ba166b
hg: obtain lock when creating share from pooled repo (issue5104) There are race conditions between clients performing a shared clone to pooled storage: 1) Clients race to create the new shared repo in the pool directory 2) 1 client is seeding the repo in the pool directory and another goes to share it before it is fully cloned We prevent these race conditions by obtaining a lock in the pool directory that is derived from the name of the repo we will be accessing. To test this, a simple generic "lockdelay" extension has been added. The extension inserts an optional, configurable delay before or after lock acquisition. In the test, we delay 2 seconds after lock acquisition in the first process and 1 second before lock acquisition in the 2nd process. This means the first process has 1s to obtain the lock. There is a race condition here. If we encounter it in the wild, we could change the dummy extension to wait on the lock file to appear instead of relying on timing. But that's more complicated. Let's see what happens first.
mercurial/hg.py
tests/lockdelay.py
tests/test-clone.t
--- a/mercurial/hg.py	Tue Mar 01 03:28:46 2016 +0900
+++ b/mercurial/hg.py	Sat Feb 27 18:22:49 2016 -0800
@@ -335,17 +335,30 @@
                                "support clone by revision"))
         revs = [srcpeer.lookup(r) for r in rev]
 
+    # Obtain a lock before checking for or cloning the pooled repo otherwise
+    # 2 clients may race creating or populating it.
+    pooldir = os.path.dirname(sharepath)
+    # lock class requires the directory to exist.
+    try:
+        util.makedir(pooldir, False)
+    except OSError as e:
+        if e.errno != errno.EEXIST:
+            raise
+
+    poolvfs = scmutil.vfs(pooldir)
     basename = os.path.basename(sharepath)
 
-    if os.path.exists(sharepath):
-        ui.status(_('(sharing from existing pooled repository %s)\n') %
-                  basename)
-    else:
-        ui.status(_('(sharing from new pooled repository %s)\n') % basename)
-        # Always use pull mode because hardlinks in share mode don't work well.
-        # Never update because working copies aren't necessary in share mode.
-        clone(ui, peeropts, source, dest=sharepath, pull=True,
-              rev=rev, update=False, stream=stream)
+    with lock.lock(poolvfs, '%s.lock' % basename):
+        if os.path.exists(sharepath):
+            ui.status(_('(sharing from existing pooled repository %s)\n') %
+                      basename)
+        else:
+            ui.status(_('(sharing from new pooled repository %s)\n') % basename)
+            # Always use pull mode because hardlinks in share mode don't work
+            # well. Never update because working copies aren't necessary in
+            # share mode.
+            clone(ui, peeropts, source, dest=sharepath, pull=True,
+                  rev=rev, update=False, stream=stream)
 
     sharerepo = repository(ui, path=sharepath)
     share(ui, sharerepo, dest=dest, update=update, bookmarks=False)
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/lockdelay.py	Sat Feb 27 18:22:49 2016 -0800
@@ -0,0 +1,26 @@
+# Dummy extension that adds a delay after acquiring a lock.
+#
+# This extension can be used to test race conditions between lock acquisition.
+
+from __future__ import absolute_import
+
+import os
+import time
+
+from mercurial import (
+    lock as lockmod,
+)
+
+class delaylock(lockmod.lock):
+    def lock(self):
+        delay = float(os.environ.get('HGPRELOCKDELAY', '0.0'))
+        if delay:
+            time.sleep(delay)
+        res = super(delaylock, self).lock()
+        delay = float(os.environ.get('HGPOSTLOCKDELAY', '0.0'))
+        if delay:
+            time.sleep(delay)
+        return res
+
+def extsetup(ui):
+    lockmod.lock = delaylock
--- a/tests/test-clone.t	Tue Mar 01 03:28:46 2016 +0900
+++ b/tests/test-clone.t	Sat Feb 27 18:22:49 2016 -0800
@@ -1025,3 +1025,49 @@
   $ hg --config share.pool=share -q clone -e "python \"$TESTDIR/dummyssh\"" a ssh://user@dummy/remote
   $ hg -R remote id -r 0
   acb14030fe0a
+
+Cloning into pooled storage doesn't race (issue5104)
+
+  $ HGPOSTLOCKDELAY=2.0 hg --config share.pool=racepool --config extensions.lockdelay=$TESTDIR/lockdelay.py clone source1a share-destrace1 > race1.log 2>&1 &
+  $ HGPRELOCKDELAY=1.0 hg --config share.pool=racepool --config extensions.lockdelay=$TESTDIR/lockdelay.py clone source1a share-destrace2  > race2.log 2>&1
+  $ wait
+
+  $ hg -R share-destrace1 log -r tip
+  changeset:   2:e5bfe23c0b47
+  bookmark:    bookA
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     1a
+  
+
+  $ hg -R share-destrace2 log -r tip
+  changeset:   2:e5bfe23c0b47
+  bookmark:    bookA
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     1a
+  
+  $ cat race1.log
+  (sharing from new pooled repository b5f04eac9d8f7a6a9fcb070243cccea7dc5ea0c1)
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 3 changesets with 3 changes to 1 files
+  updating working directory
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  searching for changes
+  no changes found
+  adding remote bookmark bookA
+
+  $ cat race2.log
+  (sharing from existing pooled repository b5f04eac9d8f7a6a9fcb070243cccea7dc5ea0c1)
+  updating working directory
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  waiting for lock on repository share-destrace2 held by * (glob)
+  got lock after \d+ seconds (re)
+  searching for changes
+  no changes found
+  adding remote bookmark bookA