Patchwork [3,of,4,marmoute-reviewed] lock.release: don't call postrelease functions for inherited locks

login
register
mail settings
Submitter Siddharth Agarwal
Date Oct. 5, 2015, 3:23 a.m.
Message ID <5864aca0e2d7597135a8.1444015393@dev6666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/10804/
State Accepted
Headers show

Comments

Siddharth Agarwal - Oct. 5, 2015, 3:23 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1444014284 25200
#      Sun Oct 04 20:04:44 2015 -0700
# Node ID 5864aca0e2d7597135a8cd061358a6fd3144325c
# Parent  7ec3a1cc6f54cb2f94a199b2381d724ec071ba5d
lock.release: don't call postrelease functions for inherited locks

Review feedback from Pierre-Yves David. The postrelease functions typically
assume the lock is not held at all.
Pierre-Yves David - Oct. 5, 2015, 3:54 a.m.
On 10/04/2015 08:23 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1444014284 25200
> #      Sun Oct 04 20:04:44 2015 -0700
> # Node ID 5864aca0e2d7597135a8cd061358a6fd3144325c
> # Parent  7ec3a1cc6f54cb2f94a199b2381d724ec071ba5d
> lock.release: don't call postrelease functions for inherited locks
>
> Review feedback from Pierre-Yves David. The postrelease functions typically
> assume the lock is not held at all.

I've moved that last sentence as an inline comment.

> diff --git a/mercurial/lock.py b/mercurial/lock.py
> --- a/mercurial/lock.py
> +++ b/mercurial/lock.py
> @@ -221,8 +221,9 @@ class lock(object):
>                           self.vfs.unlink(self.f)
>                       except OSError:
>                           pass
> -            for callback in self.postrelease:
> -                callback()
> +            if not self._parentheld:
> +                for callback in self.postrelease:
> +                    callback()

Patch

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -221,8 +221,9 @@  class lock(object):
                         self.vfs.unlink(self.f)
                     except OSError:
                         pass
-            for callback in self.postrelease:
-                callback()
+            if not self._parentheld:
+                for callback in self.postrelease:
+                    callback()
 
 def release(*locks):
     for lock in locks:
diff --git a/tests/test-lock.py b/tests/test-lock.py
--- a/tests/test-lock.py
+++ b/tests/test-lock.py
@@ -169,7 +169,7 @@  class testlock(unittest.TestCase):
 
             childlock.release()
             childstate.assertreleasecalled(True)
-            childstate.assertpostreleasecalled(True)
+            childstate.assertpostreleasecalled(False)
             childstate.assertlockexists(True)
 
             parentstate.resetacquirefn()
@@ -208,7 +208,7 @@  class testlock(unittest.TestCase):
 
                 lock2.release()
                 state2.assertreleasecalled(True)
-                state2.assertpostreleasecalled(True)
+                state2.assertpostreleasecalled(False)
                 state2.assertlockexists(True)
 
                 state1.resetacquirefn()
@@ -217,7 +217,7 @@  class testlock(unittest.TestCase):
 
             lock1.release()
             state1.assertreleasecalled(True)
-            state1.assertpostreleasecalled(True)
+            state1.assertpostreleasecalled(False)
             state1.assertlockexists(True)
 
         lock0.release()
@@ -245,7 +245,7 @@  class testlock(unittest.TestCase):
             # release the child lock
             childlock.release()
             childstate.assertreleasecalled(True)
-            childstate.assertpostreleasecalled(True)
+            childstate.assertpostreleasecalled(False)
             childstate.assertlockexists(True)
 
         parentlock.release()