Patchwork [2,of,2,STABLE] afterlock: add the callback to the top level lock (issue4608)

login
register
mail settings
Submitter Pierre-Yves David
Date April 20, 2015, 2:15 p.m.
Message ID <59054499d89e21d7725f.1429539304@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8745/
State Accepted
Headers show

Comments

Pierre-Yves David - April 20, 2015, 2:15 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1429536475 -7200
#      Mon Apr 20 15:27:55 2015 +0200
# Branch stable
# Node ID 59054499d89e21d7725f851c755ab4392dc0c1e2
# Parent  76235b47af463ebd0979f3ecb0bca2731325606f
afterlock: add the callback to the top level lock (issue4608)

If 'wlock' is taken, we should add 'afterlock' callback to the 'wlock' instead.
Otherwise, running post transaction hook after 'lock' is release but 'wlock' is
still taken lead to a deadlock (eg: 'hg update' during a hook).

This situation is much more common since: 5dc5cd7abbf5

  push: acquire local 'wlock' if "pushback" is expected (BC) (issue4596)
Matt Mackall - April 20, 2015, 4:43 p.m.
On Mon, 2015-04-20 at 16:15 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1429536475 -7200
> #      Mon Apr 20 15:27:55 2015 +0200
> # Branch stable
> # Node ID 59054499d89e21d7725f851c755ab4392dc0c1e2
> # Parent  76235b47af463ebd0979f3ecb0bca2731325606f
> afterlock: add the callback to the top level lock (issue4608)

These are queued for stable, thanks.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1192,17 +1192,20 @@  class localrepository(object):
         if acquirefn:
             acquirefn()
         return l
 
     def _afterlock(self, callback):
-        """add a callback to the current repository lock.
+        """add a callback to be run when the repository if fully unlocked
 
-        The callback will be executed on lock release."""
-        l = self._lockref and self._lockref()
-        if l:
-            l.postrelease.append(callback)
-        else:
+        The callback will be executed when the "top most" lock is released
+        (with wlock being higher level than 'lock')."""
+        for ref in (self._wlockref, self._lockref):
+            l = ref and ref()
+            if l and l.held:
+                l.postrelease.append(callback)
+                break
+        else: # no lock have been found.
             callback()
 
     def lock(self, wait=True):
         '''Lock the repository store (.hg/store) and return a weak reference
         to the lock. Use this before modifying the store (e.g. committing or
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -1,6 +1,7 @@ 
 commit hooks can see env vars
+(and post-transaction one are run unlocked)
 
   $ hg init a
   $ cd a
   $ cat > .hg/hgrc <<EOF
   > [hooks]
@@ -14,10 +15,11 @@  commit hooks can see env vars
   > post-cat = python "$TESTDIR/printenv.py" post-cat
   > pretxnopen = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" pretxnopen"
   > pretxnclose = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" pretxnclose"
   > txnclose = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" txnclose"
   > txnabort = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" txnabort"
+  > txnclose.checklock = hg debuglock > /dev/null
   > EOF
   $ echo a > a
   $ hg add a
   $ hg commit -m a
   precommit hook: HG_PARENT1=0000000000000000000000000000000000000000