Patchwork D7198: lock: refactor in preparation for next commit

login
register
mail settings
Submitter phabricator
Date Nov. 2, 2019, 1:51 a.m.
Message ID <differential-rev-PHID-DREV-qlmuousi2djmxkegwzah-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42678/
State Superseded
Headers show

Comments

phabricator - Nov. 2, 2019, 1:51 a.m.
valentin.gatienbaron created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7198

AFFECTED FILES
  mercurial/lock.py

CHANGE DETAILS




To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 13, 2019, 1:17 p.m.
pulkit added a comment.
pulkit added subscribers: marmoute, pulkit.


  @marmoute can you have a look at these two patches?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7198/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7198

To: valentin.gatienbaron, #hg-reviewers
Cc: pulkit, marmoute, mercurial-devel
phabricator - Nov. 13, 2019, 2:21 p.m.
marmoute added a comment.


  In D7198#108404 <https://phab.mercurial-scm.org/D7198#108404>, @pulkit wrote:
  
  > @marmoute can you have a look at these two patches?
  
  Yes, I will.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7198/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7198

To: valentin.gatienbaron, #hg-reviewers
Cc: pulkit, marmoute, mercurial-devel
phabricator - Nov. 19, 2019, 3:53 a.m.
This revision is now accepted and ready to land.
indygreg added a comment.
indygreg accepted this revision.


  This looks like a non-behavior-changing refactor. Thanks also for the switch to a context manager, which is more robust.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7198/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7198

To: valentin.gatienbaron, #hg-reviewers, indygreg
Cc: indygreg, pulkit, marmoute, mercurial-devel

Patch

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -330,27 +330,32 @@ 
                 return None
             raise
 
-    def _testlock(self, locker):
+    def _lockshouldbebroken(self, locker):
         if locker is None:
-            return None
+            return False
         try:
             host, pid = locker.split(b":", 1)
         except ValueError:
-            return locker
+            return False
         if host != lock._host:
-            return locker
+            return False
         try:
             pid = int(pid)
         except ValueError:
-            return locker
+            return False
         if procutil.testpid(pid):
+            return False
+        return True
+
+    def _testlock(self, locker):
+        if not self._lockshouldbebroken(locker):
             return locker
+
         # if locker dead, break lock.  must do this with another lock
         # held, or can race and break valid lock.
         try:
-            l = lock(self.vfs, self.f + b'.break', timeout=0)
-            self.vfs.unlink(self.f)
-            l.release()
+            with lock(self.vfs, self.f + b'.break', timeout=0):
+                self.vfs.unlink(self.f)
         except error.LockError:
             return locker