[PATCH] repair: fix transaction with no lock case in strip

Laurent Charignon lcharignon at fb.com
Mon Dec 28 22:20:16 UTC 2015


# HG changeset patch
# User Laurent Charignon <lcharignon at fb.com>
# Date 1451341019 28800
#      Mon Dec 28 14:16:59 2015 -0800
# Node ID bef9c27c77012ad82dc954f97205c6042793d558
# Parent  23541bdd1610c08af247f9c8719045cf247ce541
repair: fix transaction with no lock case in strip

Before this patch, if repair.strip was called without holding a lock,
the strip transaction would be made without lock. There was no occurrences of
the problem in core (because we are always calling repair.strip while holding
a lock), so I added a test with a test extension that shows the issue.

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -158,11 +158,12 @@ def strip(ui, repo, nodelist, backup=Tru
         del curtr  # avoid carrying reference to transaction for nothing
         msg = _('programming error: cannot strip from inside a transaction')
         raise error.Abort(msg, hint=_('contact your extension maintainer'))
-
-    tr = repo.transaction("strip")
-    offset = len(tr.entries)
-
+    tr = lock = wlock = None
     try:
+        wlock = repo.wlock()
+        lock = repo.lock()
+        tr = repo.transaction("strip")
+        offset = len(tr.entries)
         tr.startgroup()
         cl.strip(striprev, tr)
         mfst.strip(striprev, tr)
@@ -205,15 +206,13 @@ def strip(ui, repo, nodelist, backup=Tru
 
         for m in updatebm:
             bm[m] = repo[newbmtarget].node()
-        lock = tr = None
+        tr = None
         try:
-            lock = repo.lock()
             tr = repo.transaction('repair')
             bm.recordchange(tr)
             tr.close()
         finally:
             tr.release()
-            lock.release()
 
         # remove undo files
         for undovfs, undofile in repo.undofiles():
@@ -236,6 +235,11 @@ def strip(ui, repo, nodelist, backup=Tru
         if saveheads or savebases:
             # Remove partial backup only if there were no exceptions
             vfs.unlink(chgrpfile)
+    finally:
+        if lock is not None:
+            lock.release()
+        if wlock is not None:
+            wlock.release()
 
     repo.destroyed()
 
diff --git a/tests/test-strip.t b/tests/test-strip.t
--- a/tests/test-strip.t
+++ b/tests/test-strip.t
@@ -858,4 +858,18 @@ Error during post-close callback of the 
   abort: boom
   [255]
 
+Wrapping strip does not require to hold a lock
 
+  $ cat > ../lockstrip.py << EOF
+  > from mercurial import cmdutil, repair
+  > 
+  > cmdtable = {}
+  > command = cmdutil.command(cmdtable)
+  > 
+  > @command('testlockstrip')
+  > def testlock(ui, repo):
+  >     repair.strip(repo.ui, repo, [repo['.'].node()])
+  > EOF
+  $ hg testlockstrip --config extensions.lockstrip=$TESTTMP/lockstrip.py
+  saved backup bundle to * (glob)
+


More information about the Mercurial-devel mailing list