Rev 5629: (spiv) Fix LockContention error when pushing new tags to bound branch in file:///home/pqm/archives/thelove/bzr/2.3/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Mar 21 05:03:38 UTC 2011


At file:///home/pqm/archives/thelove/bzr/2.3/

------------------------------------------------------------
revno: 5629 [merge]
revision-id: pqm at pqm.ubuntu.com-20110321050337-u4uytp6ia37ffach
parent: pqm at pqm.ubuntu.com-20110314124636-z4petx4rpbgoo4hc
parent: andrew.bennetts at canonical.com-20110318040116-e4a4vt22kd5iz531
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.3
timestamp: Mon 2011-03-21 05:03:37 +0000
message:
  (spiv) Fix LockContention error when pushing new tags to bound branch
   (#733350) (Andrew Bennetts)
modified:
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/tests/per_branch/test_branch.py testbranch.py-20050711070244-121d632bc37d7253
  bzrlib/tests/per_branch/test_push.py test_push.py-20070130153159-fhfap8uoifevg30j-1
  doc/en/release-notes/bzr-2.3.txt NEWS-20050323055033-4e00b5db738777ff
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2011-01-12 01:01:53 +0000
+++ b/bzrlib/branch.py	2011-03-18 04:01:16 +0000
@@ -94,6 +94,7 @@
         self._partial_revision_history_cache = []
         self._tags_bytes = None
         self._last_revision_info_cache = None
+        self._master_branch_cache = None
         self._merge_sorted_revisions_cache = None
         self._open_hook()
         hooks = Branch.hooks['open']
@@ -926,6 +927,7 @@
         self._revision_history_cache = None
         self._revision_id_to_revno_cache = None
         self._last_revision_info_cache = None
+        self._master_branch_cache = None
         self._merge_sorted_revisions_cache = None
         self._partial_revision_history_cache = []
         self._partial_revision_id_to_revno_cache = {}
@@ -2671,8 +2673,7 @@
             target.update_revisions(self, stop_revision,
                 overwrite=overwrite, graph=graph)
         if self._push_should_merge_tags():
-            result.tag_conflicts = self.tags.merge_to(target.tags,
-                overwrite)
+            result.tag_conflicts = self.tags.merge_to(target.tags, overwrite)
         result.new_revno, result.new_revid = target.last_revision_info()
         return result
 
@@ -2723,12 +2724,13 @@
         """Return the branch we are bound to.
 
         :return: Either a Branch, or None
-
-        This could memoise the branch, but if thats done
-        it must be revalidated on each new lock.
-        So for now we just don't memoise it.
-        # RBC 20060304 review this decision.
         """
+        if self._master_branch_cache is None:
+            self._master_branch_cache = self._get_master_branch(
+                possible_transports)
+        return self._master_branch_cache
+
+    def _get_master_branch(self, possible_transports):
         bound_loc = self.get_bound_location()
         if not bound_loc:
             return None
@@ -2745,6 +2747,7 @@
 
         :param location: URL to the target branch
         """
+        self._master_branch_cache = None
         if location:
             self._transport.put_bytes('bound', location+'\n',
                 mode=self.bzrdir._get_file_mode())
@@ -3002,6 +3005,7 @@
 
     def set_bound_location(self, location):
         """See Branch.set_push_location."""
+        self._master_branch_cache = None
         result = None
         config = self.get_config()
         if location is None:

=== modified file 'bzrlib/tests/per_branch/test_branch.py'
--- a/bzrlib/tests/per_branch/test_branch.py	2011-01-27 14:27:18 +0000
+++ b/bzrlib/tests/per_branch/test_branch.py	2011-03-18 04:01:16 +0000
@@ -746,6 +746,73 @@
         except errors.UpgradeRequired:
             raise tests.TestNotApplicable('Format does not support binding')
 
+    def test_unbind_clears_cached_master_branch(self):
+        """b.get_master_branch may return a cached branch if b is locked.  That
+        cache should be cleared if the master branch is changed via unbind.
+        """
+        master = self.make_branch('master')
+        branch = self.make_branch('branch')
+        try:
+            branch.bind(master)
+        except errors.UpgradeRequired:
+            raise tests.TestNotApplicable('Format does not support binding')
+        self.addCleanup(branch.lock_write().unlock)
+        self.assertNotEqual(None, branch.get_master_branch())
+        branch.unbind()
+        self.assertEqual(None, branch.get_master_branch())
+
+    def test_unlocked_does_not_cache_master_branch(self):
+        """Unlocked branches do not cache the result of get_master_branch."""
+        master = self.make_branch('master')
+        branch1 = self.make_branch('branch')
+        try:
+            branch1.bind(master)
+        except errors.UpgradeRequired:
+            raise tests.TestNotApplicable('Format does not support binding')
+        # Open branch1 again
+        branch2 = branch1.bzrdir.open_branch()
+        self.assertNotEqual(None, branch1.get_master_branch())
+        # Unbind the branch via branch2.  branch1 isn't locked so will
+        # immediately return the new value for get_master_branch.
+        branch2.unbind()
+        self.assertEqual(None, branch1.get_master_branch())
+
+    def test_bind_clears_cached_master_branch(self):
+        """b.get_master_branch may return a cached branch if b is locked.  That
+        cache should be cleared if the master branch is changed via
+        set_bound_location.
+        """
+        master1 = self.make_branch('master1')
+        master2 = self.make_branch('master2')
+        branch = self.make_branch('branch')
+        try:
+            branch.bind(master1)
+        except errors.UpgradeRequired:
+            raise tests.TestNotApplicable('Format does not support binding')
+        self.addCleanup(branch.lock_write().unlock)
+        self.assertNotEqual(None, branch.get_master_branch())
+        branch.bind(master2)
+        self.assertEqual('.', urlutils.relative_url(self.get_url('master2'),
+                branch.get_master_branch().base))
+
+    def test_set_bound_location_clears_cached_master_branch(self):
+        """b.get_master_branch may return a cached branch if b is locked.  That
+        cache should be cleared if the master branch is changed via
+        set_bound_location.
+        """
+        master1 = self.make_branch('master1')
+        master2 = self.make_branch('master2')
+        branch = self.make_branch('branch')
+        try:
+            branch.bind(master1)
+        except errors.UpgradeRequired:
+            raise tests.TestNotApplicable('Format does not support binding')
+        self.addCleanup(branch.lock_write().unlock)
+        self.assertNotEqual(None, branch.get_master_branch())
+        branch.set_bound_location(self.get_url('master2'))
+        self.assertEqual('.', urlutils.relative_url(self.get_url('master2'),
+                branch.get_master_branch().base))
+
 
 class TestStrict(per_branch.TestCaseWithBranch):
 

=== modified file 'bzrlib/tests/per_branch/test_push.py'
--- a/bzrlib/tests/per_branch/test_push.py	2011-01-13 01:02:53 +0000
+++ b/bzrlib/tests/per_branch/test_push.py	2011-03-17 06:45:35 +0000
@@ -114,6 +114,23 @@
         self.assertRaises(errors.BoundBranchConnectionFailure,
                 other.branch.push, checkout.branch)
 
+    def test_push_new_tag_to_bound_branch(self):
+        master = self.make_branch('master')
+        bound = self.make_branch('bound')
+        try:
+            bound.bind(master)
+        except errors.UpgradeRequired:
+            raise tests.TestNotApplicable(
+                'Format does not support bound branches')
+        other = bound.bzrdir.sprout('other').open_branch()
+        try:
+            other.tags.set_tag('new-tag', 'some-rev')
+        except errors.TagsNotSupported:
+            raise tests.TestNotApplicable('Format does not support tags')
+        other.push(bound)
+        self.assertEqual({'new-tag': 'some-rev'}, bound.tags.get_tag_dict())
+        self.assertEqual({'new-tag': 'some-rev'}, master.tags.get_tag_dict())
+
     def test_push_uses_read_lock(self):
         """Push should only need a read lock on the source side."""
         source = self.make_branch_and_tree('source')

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- a/doc/en/release-notes/bzr-2.3.txt	2011-03-14 10:19:22 +0000
+++ b/doc/en/release-notes/bzr-2.3.txt	2011-03-18 01:46:22 +0000
@@ -32,6 +32,10 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Fix "Unable to obtain lock" error when pushing to a bound branch if tags
+  had changed.  Bazaar was attempting to open and lock the master branch
+  twice in this case.  (Andrew Bennetts, #733350)
+
 Documentation
 *************
 




More information about the bazaar-commits mailing list