Rev 2362: (broken) Change fcntl locks to be properly exclusive within the same process. in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking
John Arbash Meinel
john at arbash-meinel.com
Tue Mar 13 23:41:18 GMT 2007
At http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking
------------------------------------------------------------
revno: 2362
revision-id: john at arbash-meinel.com-20070313233914-bigtq3yxnmf90d9s
parent: john at arbash-meinel.com-20070313231812-h74g1zz32v12lv6s
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: locking
timestamp: Tue 2007-03-13 17:39:14 -0600
message:
(broken) Change fcntl locks to be properly exclusive within the same process.
This exposes the location where we are opening the same WorkingTree in both read and write
mode, which breaks several tests.
modified:
bzrlib/lock.py lock.py-20050527050856-ec090bb51bc03349
bzrlib/tests/test_lock.py test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
-------------- next part --------------
=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py 2007-03-13 23:18:12 +0000
+++ b/bzrlib/lock.py 2007-03-13 23:39:14 +0000
@@ -113,20 +113,22 @@
class _fcntl_WriteLock(_fcntl_FileLock):
- open_locks = {}
+ _open_locks = set()
def __init__(self, filename):
# standard IO errors get exposed directly.
super(_fcntl_WriteLock, self).__init__()
+ self.filename = realpath(filename)
+ if (self.filename in _fcntl_WriteLock._open_locks
+ or self.filename in _fcntl_ReadLock._open_locks):
+ self._clear_f()
+ raise errors.LockContention(self.filename)
+
self._open(filename, 'rb+')
- self.filename = realpath(filename)
- if self.filename in self.open_locks:
- self._clear_f()
- raise LockContention(self.filename)
# reserve a slot for this lock - even if the lockf call fails,
# at thisi point unlock() will be called, because self.f is set.
# TODO: make this fully threadsafe, if we decide we care.
- self.open_locks[self.filename] = self.filename
+ _fcntl_WriteLock._open_locks.add(self.filename)
try:
# LOCK_NB will cause IOError to be raised if we can't grab a
# lock right away.
@@ -137,19 +139,24 @@
self.unlock()
# we should be more precise about whats a locking
# error and whats a random-other error
- raise LockError(e)
+ raise errors.LockError(e)
def unlock(self):
- del self.open_locks[self.filename]
+ _fcntl_WriteLock._open_locks.remove(self.filename)
self._unlock()
class _fcntl_ReadLock(_fcntl_FileLock):
- open_locks = {}
+ _open_locks = {}
def __init__(self, filename):
super(_fcntl_ReadLock, self).__init__()
+ self.filename = realpath(filename)
+ if self.filename in _fcntl_WriteLock._open_locks:
+ raise errors.LockContention(self.filename)
+ _fcntl_ReadLock._open_locks.setdefault(self.filename, 0)
+ _fcntl_ReadLock._open_locks[self.filename] += 1
self._open(filename, 'rb')
try:
# LOCK_NB will cause IOError to be raised if we can't grab a
@@ -158,9 +165,14 @@
except IOError, e:
# we should be more precise about whats a locking
# error and whats a random-other error
- raise LockError(e)
+ raise errors.LockError(e)
def unlock(self):
+ count = _fcntl_ReadLock._open_locks[self.filename]
+ if count == 1:
+ del _fcntl_ReadLock._open_locks[self.filename]
+ else:
+ _fcntl_ReadLock._open_locks[self.filename] = count - 1
self._unlock()
=== modified file 'bzrlib/tests/test_lock.py'
--- a/bzrlib/tests/test_lock.py 2007-03-13 23:11:51 +0000
+++ b/bzrlib/tests/test_lock.py 2007-03-13 23:39:14 +0000
@@ -99,14 +99,21 @@
# Taking out a lock on a locked file should raise LockContention
self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
- def _disabled_test_read_then_write_excludes(self):
+ def test_read_then_write_excludes(self):
"""If a file is read-locked, taking out a write lock should fail."""
a_lock = lock.ReadLock('a-file')
self.addCleanup(a_lock.unlock)
# Taking out a lock on a locked file should raise LockContention
self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
- def _disabled_test_write_then_read_excludes(self):
+ def test_read_unlock_write(self):
+ """Make sure that unlocking allows us to lock write"""
+ a_lock = lock.ReadLock('a-file')
+ a_lock.unlock()
+ a_lock = lock.WriteLock('a-file')
+ a_lock.unlock()
+
+ def test_write_then_read_excludes(self):
"""If a file is write-locked, taking out a read lock should fail.
The file is exclusively owned by the write lock, so we shouldn't be
@@ -116,3 +123,34 @@
self.addCleanup(a_lock.unlock)
# Taking out a lock on a locked file should raise LockContention
self.assertRaises(errors.LockContention, lock.ReadLock, 'a-file')
+
+ def test_write_unlock_read(self):
+ """If we have removed the write lock, we can grab a read lock."""
+ a_lock = lock.WriteLock('a-file')
+ a_lock.unlock()
+ a_lock = lock.ReadLock('a-file')
+ a_lock.unlock()
+
+ def test_multiple_read_unlock_write(self):
+ """We can only grab a write lock if all read locks are done."""
+ a_lock = b_lock = c_lock = None
+ try:
+ a_lock = lock.ReadLock('a-file')
+ b_lock = lock.ReadLock('a-file')
+ self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+ a_lock.unlock()
+ a_lock = None
+ self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+ b_lock.unlock()
+ b_lock = None
+ c_lock = lock.WriteLock('a-file')
+ c_lock.unlock()
+ c_lock = None
+ finally:
+ # Cleanup as needed
+ if a_lock is not None:
+ a_lock.unlock()
+ if b_lock is not None:
+ b_lock.unlock()
+ if c_lock is not None:
+ c_lock.unlock()
More information about the bazaar-commits
mailing list