Rev 2564: Add test and fix for locking robustly when rename of directories doesn't act as a mutex (thank John) in http://sourcefrog.net/bzr/dlock
mbp at sourcefrog.net
Thu Jun 28 07:06:31 BST 2007
revision-id: mbp at sourcefrog.net-20070628060630-n7ygr23xhjcnay9w
parent: mbp at sourcefrog.net-20070628060052-2st5avps6i86neat
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: dlock
timestamp: Thu 2007-06-28 16:06:30 +1000
Add test and fix for locking robustly when rename of directories doesn't act as a mutex (thank John)
=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py 2007-06-27 09:22:40 +0000
+++ b/bzrlib/lockdir.py 2007-06-28 06:06:30 +0000
@@ -194,6 +194,7 @@
:return: The lock token.
+ :raises LockContention: if the lock is held by someone else.
@@ -202,7 +203,7 @@
start_time = time.time()
- tmpname = '%s/pending.%s.tmp' % (self.path, rand_chars(20))
+ tmpname = '%s/%s.tmp' % (self.path, rand_chars(10))
@@ -210,7 +211,6 @@
# which is okay, it will be caught later and determined
# to be a LockContention.
# After creating the lock directory, try again
@@ -224,11 +224,20 @@
+ # We must check we really got the lock, because Launchpad's sftp server at one
+ # time had a bug were the rename would successfully move the new
+ # directory into the existing directory, which was incorrect.
+ # It's possible some other servers or filesystems will have a
+ # similar bug allowing someone to think they got the lock when
+ # it's already held.
+ info = self.peek()
+ if info['nonce'] != self.nonce:
+ raise errors.LockError("%s: rename succeeded, "
+ "but lock is still held by someone else" % (self,))
+ # we don't call confirm here because we don't want to set
+ # _lock_held til we're sure it's true, and because it's really a
+ # problem, not just regular contention, if this fails
self._lock_held = True
- # we used to do self.confirm() at this point, but it's really
- # unnecessary, we have no substantial chance of having it broken
- # just as it's acquired, and we believe that this lock design is
- # safe on all platforms.
# FIXME: we should remove the pending lock if we fail,
=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py 2007-06-27 09:17:22 +0000
+++ b/bzrlib/tests/test_lockdir.py 2007-06-28 06:06:30 +0000
@@ -27,6 +27,7 @@
from bzrlib.errors import (
@@ -633,3 +634,17 @@
ld2 = self.get_lock()
t2 = ld2.lock_write(token)
+ def test_lock_with_buggy_rename(self):
+ # test that lock acquisition handles servers which pretend they
+ # renamed correctly but that actually fail
+ t = transport.get_transport('brokenrename+' + self.get_url())
+ ld1 = LockDir(t, 'test_lock')
+ ld2 = LockDir(t, 'test_lock')
+ # we should notice that we failed to lock, even though the transport
+ # has the wrong rename behaviour
+ e = self.assertRaises(errors.LockError, ld2.attempt_lock)
+ "rename succeeded, but lock is still held by someone else")
More information about the bazaar-commits