Mercurial > hg-stable
changeset 45527:9b16bb3b2349
locking: remove support for inheriting locks in subprocess
This seems to have been added for merge driver, and since merge driver
is now gone...
Differential Revision: https://phab.mercurial-scm.org/D9053
| author | Martin von Zweigbergk <martinvonz@google.com> |
|---|---|
| date | Fri, 18 Sep 2020 08:27:43 -0700 |
| parents | 32ce4cbaec4b |
| children | 5eee6f4f3d0d |
| files | mercurial/localrepo.py mercurial/lock.py mercurial/scmutil.py tests/test-lock.py |
| diffstat | 4 files changed, 9 insertions(+), 219 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/localrepo.py Thu Sep 17 22:34:36 2020 -0700 +++ b/mercurial/localrepo.py Fri Sep 18 08:27:43 2020 -0700 @@ -2678,22 +2678,8 @@ ce.refresh() def _lock( - self, - vfs, - lockname, - wait, - releasefn, - acquirefn, - desc, - inheritchecker=None, - parentenvvar=None, + self, vfs, lockname, wait, releasefn, acquirefn, desc, ): - parentlock = None - # the contents of parentenvvar are used by the underlying lock to - # determine whether it can be inherited - if parentenvvar is not None: - parentlock = encoding.environ.get(parentenvvar) - timeout = 0 warntimeout = 0 if wait: @@ -2711,8 +2697,6 @@ releasefn=releasefn, acquirefn=acquirefn, desc=desc, - inheritchecker=inheritchecker, - parentlock=parentlock, signalsafe=signalsafe, ) return l @@ -2753,12 +2737,6 @@ self._lockref = weakref.ref(l) return l - def _wlockchecktransaction(self): - if self.currenttransaction() is not None: - raise error.LockInheritanceContractViolation( - b'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. @@ -2796,8 +2774,6 @@ unlock, self.invalidatedirstate, _(b'working directory of %s') % self.origroot, - inheritchecker=self._wlockchecktransaction, - parentenvvar=b'HG_WLOCK_LOCKER', ) self._wlockref = weakref.ref(l) return l
--- a/mercurial/lock.py Thu Sep 17 22:34:36 2020 -0700 +++ b/mercurial/lock.py Fri Sep 18 08:27:43 2020 -0700 @@ -202,8 +202,6 @@ releasefn=None, acquirefn=None, desc=None, - inheritchecker=None, - parentlock=None, signalsafe=True, dolock=True, ): @@ -214,10 +212,6 @@ self.releasefn = releasefn self.acquirefn = acquirefn self.desc = desc - self._inheritchecker = inheritchecker - self.parentlock = parentlock - self._parentheld = False - self._inherited = False if signalsafe: self._maybedelayedinterrupt = _delayedinterrupt else: @@ -290,14 +284,6 @@ if locker is None: continue - # special case where a parent process holds the lock -- this - # is different from the pid being different because we do - # want the unlock and postrelease functions to be called, - # but the lockfile to not be removed. - if locker == self.parentlock: - self._parentheld = True - self.held = 1 - return locker = self._testlock(locker) if locker is not None: raise error.LockHeld( @@ -377,38 +363,6 @@ locker = self._readlock() return self._testlock(locker) - @contextlib.contextmanager - def inherit(self): - """context for the lock to be inherited by a Mercurial subprocess. - - Yields a string that will be recognized by the lock in the subprocess. - Communicating this string to the subprocess needs to be done separately - -- typically by an environment variable. - """ - if not self.held: - raise error.LockInheritanceContractViolation( - b'inherit can only be called while lock is held' - ) - if self._inherited: - raise error.LockInheritanceContractViolation( - b'inherit cannot be called while lock is already inherited' - ) - if self._inheritchecker is not None: - self._inheritchecker() - if self.releasefn: - self.releasefn() - if self._parentheld: - lockname = self.parentlock - else: - lockname = b'%s:%d' % (lock._host, self.pid) - self._inherited = True - try: - yield lockname - finally: - if self.acquirefn: - self.acquirefn() - self._inherited = False - def release(self, success=True): """release the lock and execute callback function if any @@ -425,18 +379,16 @@ if self.releasefn: self.releasefn() finally: - if not self._parentheld: - try: - self.vfs.unlink(self.f) - except OSError: - pass + try: + self.vfs.unlink(self.f) + except OSError: + pass # The postrelease functions typically assume the lock is not held # at all. - if not self._parentheld: - for callback in self.postrelease: - callback(success) - # Prevent double usage and help clear cycles. - self.postrelease = None + for callback in self.postrelease: + callback(success) + # Prevent double usage and help clear cycles. + self.postrelease = None def release(*locks):
--- a/mercurial/scmutil.py Thu Sep 17 22:34:36 2020 -0700 +++ b/mercurial/scmutil.py Fri Sep 18 08:27:43 2020 -0700 @@ -1737,29 +1737,6 @@ return data -def _locksub(repo, lock, envvar, cmd, environ=None, *args, **kwargs): - if lock is None: - raise error.LockInheritanceContractViolation( - b'lock can only be inherited while held' - ) - if environ is None: - environ = {} - with lock.inherit() as locker: - environ[envvar] = locker - return repo.ui.system(cmd, environ=environ, *args, **kwargs) - - -def wlocksub(repo, cmd, *args, **kwargs): - """run cmd as a subprocess that allows inheriting repo's wlock - - This can only be called while the wlock is held. This takes all the - arguments that ui.system does, and returns the exit code of the - subprocess.""" - return _locksub( - repo, repo.currentwlock(), b'HG_WLOCK_LOCKER', cmd, *args, **kwargs - ) - - class progress(object): def __init__(self, ui, updatebar, topic, unit=b"", total=None): self.ui = ui
--- a/tests/test-lock.py Thu Sep 17 22:34:36 2020 -0700 +++ b/tests/test-lock.py Fri Sep 18 08:27:43 2020 -0700 @@ -169,121 +169,6 @@ state.assertpostreleasecalled(True) state.assertlockexists(False) - def testinheritlock(self): - d = tempfile.mkdtemp(dir=encoding.getcwd()) - parentstate = teststate(self, d) - parentlock = parentstate.makelock() - parentstate.assertacquirecalled(True) - - # set up lock inheritance - with parentlock.inherit() as lockname: - parentstate.assertreleasecalled(True) - parentstate.assertpostreleasecalled(False) - parentstate.assertlockexists(True) - - childstate = teststate(self, d, pidoffset=1) - childlock = childstate.makelock(parentlock=lockname) - childstate.assertacquirecalled(True) - - childlock.release() - childstate.assertreleasecalled(True) - childstate.assertpostreleasecalled(False) - childstate.assertlockexists(True) - - parentstate.resetacquirefn() - - parentstate.assertacquirecalled(True) - - parentlock.release() - parentstate.assertreleasecalled(True) - parentstate.assertpostreleasecalled(True) - parentstate.assertlockexists(False) - - def testmultilock(self): - d = tempfile.mkdtemp(dir=encoding.getcwd()) - state0 = teststate(self, d) - lock0 = state0.makelock() - state0.assertacquirecalled(True) - - with lock0.inherit() as lock0name: - state0.assertreleasecalled(True) - state0.assertpostreleasecalled(False) - state0.assertlockexists(True) - - state1 = teststate(self, d, pidoffset=1) - lock1 = state1.makelock(parentlock=lock0name) - state1.assertacquirecalled(True) - - # from within lock1, acquire another lock - with lock1.inherit() as lock1name: - # since the file on disk is lock0's this should have the same - # name - self.assertEqual(lock0name, lock1name) - - state2 = teststate(self, d, pidoffset=2) - lock2 = state2.makelock(parentlock=lock1name) - state2.assertacquirecalled(True) - - lock2.release() - state2.assertreleasecalled(True) - state2.assertpostreleasecalled(False) - state2.assertlockexists(True) - - state1.resetacquirefn() - - state1.assertacquirecalled(True) - - lock1.release() - state1.assertreleasecalled(True) - state1.assertpostreleasecalled(False) - state1.assertlockexists(True) - - lock0.release() - - def testinheritlockfork(self): - d = tempfile.mkdtemp(dir=encoding.getcwd()) - parentstate = teststate(self, d) - parentlock = parentstate.makelock() - parentstate.assertacquirecalled(True) - - # set up lock inheritance - with parentlock.inherit() as lockname: - childstate = teststate(self, d, pidoffset=1) - childlock = childstate.makelock(parentlock=lockname) - childstate.assertacquirecalled(True) - - # fork the child lock - forkchildlock = copy.copy(childlock) - forkchildlock._pidoffset += 1 - forkchildlock.release() - childstate.assertreleasecalled(False) - childstate.assertpostreleasecalled(False) - childstate.assertlockexists(True) - - # release the child lock - childlock.release() - childstate.assertreleasecalled(True) - childstate.assertpostreleasecalled(False) - childstate.assertlockexists(True) - - parentlock.release() - - def testinheritcheck(self): - d = tempfile.mkdtemp(dir=encoding.getcwd()) - state = teststate(self, d) - - def check(): - raise error.LockInheritanceContractViolation('check failed') - - lock = state.makelock(inheritchecker=check) - state.assertacquirecalled(True) - - with self.assertRaises(error.LockInheritanceContractViolation): - with lock.inherit(): - pass - - lock.release() - def testfrequentlockunlock(self): """This tests whether lock acquisition fails as expected, even if (1) lock can't be acquired (makelock fails by EEXIST), and