Patchwork [3,of,5] localrepo: prevent wlock from being inherited when a transaction is running

login
register
mail settings
Submitter Siddharth Agarwal
Date Oct. 6, 2015, 11:40 p.m.
Message ID <2d3f081900f9e0eb5d58.1444174805@dev6666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/10838/
State Accepted
Headers show

Comments

Siddharth Agarwal - Oct. 6, 2015, 11:40 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1444162745 25200
#      Tue Oct 06 13:19:05 2015 -0700
# Node ID 2d3f081900f9e0eb5d583211f5db699f2f5b000b
# Parent  d790c5c34f0e0693a90e99773f48f9a8b04f14b2
localrepo: prevent wlock from being inherited when a transaction is running

Review feedback from Pierre-Yves David. A separate line of work is working to
ensure that dirstate writes are written to a separate 'pending' file while a
transaction is active. Lock inheritance currently conflicts with that, so dodge
the issue by simply preventing inheritance while a transaction is running.

Custom merge drivers aren't going to run inside a transaction, so this doesn't
affect that.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1212,7 +1212,7 @@  class localrepository(object):
             ce.refresh()
 
     def _lock(self, vfs, lockname, wait, releasefn, acquirefn, desc,
-              parentenvvar=None):
+              inheritchecker=None, parentenvvar=None):
         parentlock = None
         # the contents of parentenvvar are used by the underlying lock to
         # determine whether it can be inherited
@@ -1221,6 +1221,7 @@  class localrepository(object):
         try:
             l = lockmod.lock(vfs, lockname, 0, releasefn=releasefn,
                              acquirefn=acquirefn, desc=desc,
+                             inheritchecker=inheritchecker,
                              parentlock=parentlock)
         except error.LockHeld as inst:
             if not wait:
@@ -1265,6 +1266,11 @@  class localrepository(object):
         self._lockref = weakref.ref(l)
         return l
 
+    def _wlockchecktransaction(self):
+        if self.currenttransaction() is not None:
+            raise error.LockInheritanceContractViolation(
+                'wlock cannot be inherited in the middle of a transaction')
+
     def wlock(self, wait=True):
         '''Lock the non-store parts of the repository (everything under
         .hg except .hg/store) and return a weak reference to the lock.
@@ -1296,7 +1302,9 @@  class localrepository(object):
 
         l = self._lock(self.vfs, "wlock", wait, unlock,
                        self.invalidatedirstate, _('working directory of %s') %
-                       self.origroot, parentenvvar='HG_WLOCK_LOCKER')
+                       self.origroot,
+                       inheritchecker=self._wlockchecktransaction,
+                       parentenvvar='HG_WLOCK_LOCKER')
         self._wlockref = weakref.ref(l)
         return l
 
diff --git a/tests/test-lock.py b/tests/test-lock.py
--- a/tests/test-lock.py
+++ b/tests/test-lock.py
@@ -260,7 +260,7 @@  class testlock(unittest.TestCase):
         state.assertacquirecalled(True)
 
         def tryinherit():
-            with lock.inherit() as lockname:
+            with lock.inherit():
                 pass
 
         self.assertRaises(error.LockInheritanceContractViolation, tryinherit)