Rev 2286: LockDir, Repository and Branch lock token changes from the hpss branch. in sftp://bazaar.launchpad.net/%7Ebzr/bzr/bzr.dev.hpss.api.changes/
Andrew Bennetts
andrew.bennetts at canonical.com
Wed Apr 11 14:37:11 BST 2007
At sftp://bazaar.launchpad.net/%7Ebzr/bzr/bzr.dev.hpss.api.changes/
------------------------------------------------------------
revno: 2286
revision-id: andrew.bennetts at canonical.com-20070411133532-u6x6edf3dmzamnaq
parent: andrew.bennetts at canonical.com-20070411064005-zylli6el5cz7kwnb
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: bzr.dev.hpss.api.changes
timestamp: Wed 2007-04-11 23:35:32 +1000
message:
LockDir, Repository and Branch lock token changes from the hpss branch.
modified:
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
bzrlib/lockdir.py lockdir.py-20060220222025-98258adf27fbdda3
bzrlib/repository.py rev_storage.py-20051111201905-119e9401e46257e3
bzrlib/tests/branch_implementations/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
bzrlib/tests/lock_helpers.py LockHelpers.py-20060707151933-tav3o2hpibwi53u4-1
bzrlib/tests/repository_implementations/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
bzrlib/tests/test_lockable_files.py test_lockable_files.py-20051225183927-365c7fd99591caf1
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2007-04-04 05:19:38 +0000
+++ b/bzrlib/branch.py 2007-04-11 13:35:32 +0000
@@ -195,6 +195,22 @@
def get_physical_lock_status(self):
raise NotImplementedError(self.get_physical_lock_status)
+ def leave_lock_in_place(self):
+ """Tell this branch object not to release the physical lock when this
+ object is unlocked.
+
+ If lock_write doesn't return a token, then this method is not supported.
+ """
+ self.control_files.leave_in_place()
+
+ def dont_leave_lock_in_place(self):
+ """Tell this branch object to release the physical lock when this
+ object is unlocked, even if it didn't originally acquire it.
+
+ If lock_write doesn't return a token, then this method is not supported.
+ """
+ self.control_files.dont_leave_in_place()
+
def abspath(self, name):
"""Return absolute filename for something in the branch
@@ -1223,13 +1239,26 @@
def is_locked(self):
return self.control_files.is_locked()
- def lock_write(self):
- self.repository.lock_write()
+ def lock_write(self, tokens=None):
+ if tokens is not None:
+ branch_token, repo_token = tokens
+ else:
+ branch_token = repo_token = None
+ repo_token = self.repository.lock_write(token=repo_token)
try:
- self.control_files.lock_write()
+ branch_token = self.control_files.lock_write(token=branch_token)
except:
self.repository.unlock()
raise
+ else:
+ tokens = (branch_token, repo_token)
+ assert tokens == (None, None) or None not in tokens, (
+ 'Both branch and repository locks must return tokens, or else '
+ 'neither must return tokens. Got %r.' % (tokens,))
+ if tokens == (None, None):
+ return None
+ else:
+ return tokens
def lock_read(self):
self.repository.lock_read()
=== modified file 'bzrlib/lockable_files.py'
--- a/bzrlib/lockable_files.py 2007-03-28 07:38:53 +0000
+++ b/bzrlib/lockable_files.py 2007-04-11 13:35:32 +0000
@@ -222,6 +222,15 @@
raise errors.BzrBadParameterNotString(a_string)
self.put_bytes(path, a_string.encode('utf-8'))
+ def leave_in_place(self):
+ """Set this LockableFiles to not clear the physical lock on unlock."""
+ self._lock.leave_in_place()
+
+ def dont_leave_in_place(self):
+ """Set this LockableFiles to clear the physical lock on unlock."""
+ # XXX: think about renaming this!
+ self._lock.dont_leave_in_place()
+
def lock_write(self, token=None):
"""Lock this group of files for writing.
@@ -232,12 +241,6 @@
instance doesn't support using token locks.
:raises MismatchedToken: if the specified token doesn't match the token
of the existing lock.
-
- A token should be passed in if you know that you have locked the object
- some other way, and need to synchronise this object's state with that
- fact. For instance, this could happen when accessing the same branch
- over bzr+ssh:// and then falling back to do some operations on the same
- branch via sftp://.
"""
# mutter("lock write: %s (%s)", self, self._lock_count)
# TODO: Upgrade locking to support using a Transport,
@@ -247,6 +250,7 @@
raise errors.ReadOnlyError(self)
self._lock.validate_token(token)
self._lock_count += 1
+ return self._token_from_lock
else:
token_from_lock = self._lock.lock_write(token=token)
#note('write locking %s', self)
@@ -254,6 +258,9 @@
self._lock_mode = 'w'
self._lock_count = 1
self._set_transaction(transactions.WriteTransaction())
+ # XXX: add test for the case that requires self._token_from_lock:
+ # token = x.lock_write(); assert(x.lock_write() == token)
+ self._token_from_lock = token_from_lock
return token_from_lock
def lock_read(self):
@@ -351,6 +358,12 @@
def break_lock(self):
raise NotImplementedError(self.break_lock)
+ def leave_in_place(self):
+ raise NotImplementedError(self.leave_in_place)
+
+ def dont_leave_in_place(self):
+ raise NotImplementedError(self.dont_leave_in_place)
+
def lock_write(self, token=None):
if token is not None:
raise errors.TokenLockingNotSupported(self)
=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py 2007-03-28 07:38:53 +0000
+++ b/bzrlib/lockdir.py 2007-04-11 13:35:32 +0000
@@ -166,7 +166,6 @@
self._held_info_path = self._held_dir + self.__INFO_NAME
self._file_modebits = file_modebits
self._dir_modebits = dir_modebits
- self.nonce = rand_chars(20)
self._report_function = note
@@ -210,6 +209,7 @@
# After creating the lock directory, try again
self.transport.mkdir(tmpname)
+ self.nonce = rand_chars(20)
info_bytes = self._prepare_info()
# We use put_file_non_atomic because we just created a new unique
# directory so we don't have to worry about files existing there.
@@ -419,6 +419,12 @@
time.sleep(poll)
else:
raise LockContention(self)
+
+ def leave_in_place(self):
+ self._locked_via_token = True
+
+ def dont_leave_in_place(self):
+ self._locked_via_token = False
def lock_write(self, token=None):
"""Wait for and acquire the lock.
@@ -428,21 +434,17 @@
:returns: a token if this instance supports tokens, otherwise None.
:raises TokenLockingNotSupported: when a token is given but this
instance doesn't support using token locks.
- :raises TokenMismatch: if the specified token doesn't match the token
+ :raises MismatchedToken: if the specified token doesn't match the token
of the existing lock.
-
- A token should be passed in if you know that you have locked the object
- some other way, and need to synchronise this object's state with that
- fact. For instance, this could happen when accessing the same branch
- over bzr+ssh:// and then falling back to do some operations on the same
- branch via sftp://.
XXX: docstring duplicated from LockableFiles.lock_write.
"""
if token is not None:
self.validate_token(token)
+ self.nonce = token
self._lock_held = True
self._locked_via_token = True
+ return token
else:
self.wait_lock()
return self.peek().get('nonce')
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2007-04-01 06:19:16 +0000
+++ b/bzrlib/repository.py 2007-04-11 13:35:32 +0000
@@ -231,8 +231,20 @@
def is_locked(self):
return self.control_files.is_locked()
- def lock_write(self):
- self.control_files.lock_write()
+ def lock_write(self, token=None):
+ """Lock this repository for writing.
+
+ :param token: if this is already locked, then lock_write will fail
+ unless the token matches the existing lock.
+ :returns: a token if this instance supports tokens, otherwise None.
+ :raises TokenLockingNotSupported: when a token is given but this
+ instance doesn't support using token locks.
+ :raises MismatchedToken: if the specified token doesn't match the token
+ of the existing lock.
+
+ XXX: this docstring is duplicated in many places, e.g. lockable_files.py
+ """
+ return self.control_files.lock_write(token=token)
def lock_read(self):
self.control_files.lock_read()
@@ -240,6 +252,22 @@
def get_physical_lock_status(self):
return self.control_files.get_physical_lock_status()
+ def leave_lock_in_place(self):
+ """Tell this repository not to release the physical lock when this
+ object is unlocked.
+
+ If lock_write doesn't return a token, then this method is not supported.
+ """
+ self.control_files.leave_in_place()
+
+ def dont_leave_lock_in_place(self):
+ """Tell this repository to release the physical lock when this
+ object is unlocked, even if it didn't originally acquire it.
+
+ If lock_write doesn't return a token, then this method is not supported.
+ """
+ self.control_files.dont_leave_in_place()
+
@needs_read_lock
def gather_stats(self, revid=None, committers=None):
"""Gather statistics from a revision id.
=== modified file 'bzrlib/tests/branch_implementations/test_locking.py'
--- a/bzrlib/tests/branch_implementations/test_locking.py 2007-03-28 07:33:07 +0000
+++ b/bzrlib/tests/branch_implementations/test_locking.py 2007-04-11 13:35:32 +0000
@@ -227,6 +227,198 @@
('rc', 'ul', True),
], self.locks)
+ def test_lock_write_returns_None_refuses_tokens(self):
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is not None:
+ # This test does not apply, because this lockable supports
+ # tokens.
+ return
+ self.assertRaises(errors.TokenLockingNotSupported,
+ branch.lock_write, tokens=('token','token'))
+ finally:
+ branch.unlock()
+
+ def test_reentering_lock_write_raises_on_token_mismatch(self):
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ branch_token, repo_token = tokens
+ different_branch_token = branch_token + 'xxx'
+ different_repo_token = repo_token + 'xxx'
+ # Re-using the same lockable instance with a different branch token
+ # will raise TokenMismatch.
+ self.assertRaises(errors.TokenMismatch,
+ branch.lock_write,
+ tokens=(different_branch_token, repo_token))
+ # Similarly for a different repository token.
+ self.assertRaises(errors.TokenMismatch,
+ branch.lock_write,
+ tokens=(branch_token, different_repo_token))
+ finally:
+ branch.unlock()
+
+ def test_lock_write_with_nonmatching_token(self):
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this branch refuses
+ # tokens.
+ return
+ branch_token, repo_token = tokens
+ different_branch_token = branch_token + 'xxx'
+ different_repo_token = repo_token + 'xxx'
+
+ new_branch = branch.bzrdir.open_branch()
+ # We only want to test the relocking abilities of branch, so use the
+ # existing repository object which is already locked.
+ new_branch.repository = branch.repository
+ self.assertRaises(errors.TokenMismatch,
+ new_branch.lock_write,
+ tokens=(different_branch_token, repo_token))
+ self.assertRaises(errors.TokenMismatch,
+ new_branch.lock_write,
+ tokens=(branch_token, different_repo_token))
+ finally:
+ branch.unlock()
+
+
+ def test_lock_write_with_matching_token(self):
+ """Test that a branch can be locked with a token, if it is already
+ locked by that token."""
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this branch refuses tokens.
+ return
+ # The same instance will accept a second lock_write if the specified
+ # token matches.
+ branch.lock_write(tokens=tokens)
+ branch.unlock()
+ # Calling lock_write on a new instance for the same lockable will
+ # also succeed.
+ new_branch = branch.bzrdir.open_branch()
+ # We only want to test the relocking abilities of branch, so use the
+ # existing repository object which is already locked.
+ new_branch.repository = branch.repository
+ new_branch.lock_write(tokens=tokens)
+ new_branch.unlock()
+ finally:
+ branch.unlock()
+
+ def test_unlock_after_lock_write_with_tokens(self):
+ # If lock_write did not physically acquire the lock (because it was
+ # passed some tokens), then unlock should not physically release it.
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ new_branch = branch.bzrdir.open_branch()
+ # We only want to test the relocking abilities of branch, so use the
+ # existing repository object which is already locked.
+ new_branch.repository = branch.repository
+ new_branch.lock_write(tokens=tokens)
+ new_branch.unlock()
+ self.assertTrue(branch.get_physical_lock_status()) #XXX
+ finally:
+ branch.unlock()
+
+ def test_lock_write_with_tokens_fails_when_unlocked(self):
+ # Lock and unlock to get superficially valid tokens. This mimics a
+ # likely programming error, where a caller accidentally tries to lock
+ # with tokens that are no longer valid (because the original lock was
+ # released).
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ branch.unlock()
+ if tokens is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+
+ self.assertRaises(errors.TokenMismatch,
+ branch.lock_write, tokens=tokens)
+
+ def test_lock_write_reenter_with_tokens(self):
+ branch = self.make_branch('b')
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ # Relock with a token.
+ branch.lock_write(tokens=tokens)
+ branch.unlock()
+ finally:
+ branch.unlock()
+ # The lock should be unlocked on disk. Verify that with a new lock
+ # instance.
+ new_branch = branch.bzrdir.open_branch()
+ # Calling lock_write now should work, rather than raise LockContention.
+ new_branch.lock_write()
+ new_branch.unlock()
+
+ def test_leave_lock_in_place(self):
+ branch = self.make_branch('b')
+ # Lock the branch, then use leave_lock_in_place so that when we
+ # unlock the branch the lock is still held on disk.
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this repository refuses lock
+ # tokens.
+ self.assertRaises(NotImplementedError, branch.leave_lock_in_place)
+ return
+ branch.leave_lock_in_place()
+ finally:
+ branch.unlock()
+ # We should be unable to relock the repo.
+ self.assertRaises(errors.LockContention, branch.lock_write)
+
+ def test_dont_leave_lock_in_place(self):
+ branch = self.make_branch('b')
+ # Create a lock on disk.
+ tokens = branch.lock_write()
+ try:
+ if tokens is None:
+ # This test does not apply, because this branch refuses lock
+ # tokens.
+ self.assertRaises(NotImplementedError,
+ branch.dont_leave_lock_in_place)
+ return
+ try:
+ branch.leave_lock_in_place()
+ except NotImplementedError:
+ # This branch doesn't support this API.
+ return
+ branch.repository.leave_lock_in_place()
+ finally:
+ branch.unlock()
+ # Reacquire the lock (with a different branch object) by using the
+ # tokens.
+ new_branch = branch.bzrdir.open_branch()
+ new_branch.lock_write(tokens=tokens)
+ # Call dont_leave_lock_in_place, so that the lock will be released by
+ # this instance, even though the lock wasn't originally acquired by it.
+ new_branch.dont_leave_lock_in_place()
+ new_branch.repository.dont_leave_lock_in_place()
+ new_branch.unlock()
+ # Now the branch (and repository) is unlocked. Test this by locking it
+ # without tokens.
+ branch.lock_write()
+ branch.unlock()
+
def test_lock_read_then_unlock(self):
# Calling lock_read then unlocking should work without errors.
branch = self.make_branch('b')
=== modified file 'bzrlib/tests/lock_helpers.py'
--- a/bzrlib/tests/lock_helpers.py 2007-03-28 13:33:58 +0000
+++ b/bzrlib/tests/lock_helpers.py 2007-04-11 13:35:32 +0000
@@ -56,7 +56,7 @@
return self._other.lock_read()
raise TestPreventLocking('lock_read disabled')
- def lock_write(self):
+ def lock_write(self, token=None):
self._sequence.append((self._other_id, 'lw', self._allow_write))
if self._allow_write:
return self._other.lock_write()
=== modified file 'bzrlib/tests/repository_implementations/test_repository.py'
--- a/bzrlib/tests/repository_implementations/test_repository.py 2007-04-01 06:19:16 +0000
+++ b/bzrlib/tests/repository_implementations/test_repository.py 2007-04-11 13:35:32 +0000
@@ -434,6 +434,58 @@
class TestRepositoryLocking(TestCaseWithRepository):
+ def setUp(self):
+ TestCaseWithRepository.setUp(self)
+ self.reduceLockdirTimeout()
+
+ def test_leave_lock_in_place(self):
+ repo = self.make_repository('r')
+ # Lock the repository, then use leave_lock_in_place so that when we
+ # unlock the repository the lock is still held on disk.
+ token = repo.lock_write()
+ try:
+ if token is None:
+ # This test does not apply, because this repository refuses lock
+ # tokens.
+ self.assertRaises(NotImplementedError, repo.leave_lock_in_place)
+ return
+ repo.leave_lock_in_place()
+ finally:
+ repo.unlock()
+ # We should be unable to relock the repo.
+ self.assertRaises(errors.LockContention, repo.lock_write)
+
+ def test_dont_leave_lock_in_place(self):
+ repo = self.make_repository('r')
+ # Create a lock on disk.
+ token = repo.lock_write()
+ try:
+ if token is None:
+ # This test does not apply, because this repository refuses lock
+ # tokens.
+ self.assertRaises(NotImplementedError,
+ repo.dont_leave_lock_in_place)
+ return
+ try:
+ repo.leave_lock_in_place()
+ except NotImplementedError:
+ # This repository doesn't support this API.
+ return
+ finally:
+ repo.unlock()
+ # Reacquire the lock (with a different repository object) by using the
+ # token.
+ new_repo = repo.bzrdir.open_repository()
+ new_repo.lock_write(token=token)
+ # Call dont_leave_lock_in_place, so that the lock will be released by
+ # this instance, even though the lock wasn't originally acquired by it.
+ new_repo.dont_leave_lock_in_place()
+ new_repo.unlock()
+ # Now the repository is unlocked. Test this by locking it (without a
+ # token).
+ repo.lock_write()
+ repo.unlock()
+
def test_lock_read_then_unlock(self):
# Calling lock_read then unlocking should work without errors.
repo = self.make_repository('r')
=== modified file 'bzrlib/tests/test_lockable_files.py'
--- a/bzrlib/tests/test_lockable_files.py 2007-03-28 07:08:42 +0000
+++ b/bzrlib/tests/test_lockable_files.py 2007-04-11 13:35:32 +0000
@@ -36,13 +36,7 @@
class _TestLockableFiles_mixin(object):
def setUp(self):
- # Reduce the default timeout, so that if tests fail, they will do so
- # reasonably quickly.
- orig_timeout = lockdir._DEFAULT_TIMEOUT_SECONDS
- def resetTimeout():
- lockdir._DEFAULT_TIMEOUT_SECONDS = orig_timeout
- self.addCleanup(resetTimeout)
- lockdir._DEFAULT_TIMEOUT_SECONDS = 3
+ self.reduceLockdirTimeout()
def test_read_write(self):
self.assertRaises(NoSuchFile, self.lockable.get, 'foo')
@@ -147,6 +141,22 @@
finally:
self.lockable.unlock()
+ def test_lock_write_returns_token_when_given_token(self):
+ token = self.lockable.lock_write()
+ try:
+ if token is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ new_lockable = self.get_lockable()
+ token_from_new_lockable = new_lockable.lock_write(token=token)
+ try:
+ self.assertEqual(token, token_from_new_lockable)
+ finally:
+ new_lockable.unlock()
+ finally:
+ self.lockable.unlock()
+
def test_lock_write_raises_on_token_mismatch(self):
token = self.lockable.lock_write()
try:
@@ -231,8 +241,11 @@
# tokens.
return
# Relock with a token.
- self.lockable.lock_write(token=token)
- self.lockable.unlock()
+ token_from_reentry = self.lockable.lock_write(token=token)
+ try:
+ self.assertEqual(token, token_from_reentry)
+ finally:
+ self.lockable.unlock()
finally:
self.lockable.unlock()
# The lock should be unlocked on disk. Verify that with a new lock
@@ -242,6 +255,45 @@
new_lockable.lock_write()
new_lockable.unlock()
+ def test_leave_in_place(self):
+ token = self.lockable.lock_write()
+ try:
+ if token is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ self.lockable.leave_in_place()
+ finally:
+ self.lockable.unlock()
+ # At this point, the lock is still in place on disk
+ self.assertRaises(errors.LockContention, self.lockable.lock_write)
+ # But should be relockable with a token.
+ self.lockable.lock_write(token=token)
+ self.lockable.unlock()
+
+ def test_dont_leave_in_place(self):
+ token = self.lockable.lock_write()
+ try:
+ if token is None:
+ # This test does not apply, because this lockable refuses
+ # tokens.
+ return
+ self.lockable.leave_in_place()
+ finally:
+ self.lockable.unlock()
+ # At this point, the lock is still in place on disk.
+ # Acquire the existing lock with the token, and ask that it is removed
+ # when this object unlocks, and unlock to trigger that removal.
+ new_lockable = self.get_lockable()
+ new_lockable.lock_write(token=token)
+ new_lockable.dont_leave_in_place()
+ new_lockable.unlock()
+ # At this point, the lock is no longer on disk, so we can lock it.
+ third_lockable = self.get_lockable()
+ third_lockable.lock_write()
+ third_lockable.unlock()
+
+
More information about the bazaar-commits
mailing list