Rev 2364: [merge] locking changes into bzr.dev tip in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/simple_locking

John Arbash Meinel john at arbash-meinel.com
Mon Mar 19 20:56:37 GMT 2007


At http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/simple_locking

------------------------------------------------------------
revno: 2364
revision-id: john at arbash-meinel.com-20070319205618-21plqrdf0feihonk
parent: pqm at pqm.ubuntu.com-20070317015305-7b7562331da9f786
parent: john at arbash-meinel.com-20070319204546-xxywya2iuemwqdt9
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: simple_locking
timestamp: Mon 2007-03-19 15:56:18 -0500
message:
  [merge] locking changes into bzr.dev tip
added:
  bzrlib/tests/per_lock/test_temporary_write_lock.py test_temporary_write-20070314233412-xp3ocbyvw3woa03w-1
modified:
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
  bzrlib/tests/per_lock/__init__.py __init__.py-20070314201444-u92yjsqrkh2m3qcb-1
  bzrlib/tests/per_lock/test_lock.py test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
  bzrlib/tests/test_commit_merge.py test_commit_merge.py-20050920084723-819eeeff77907bc5
  bzrlib/tests/test_dirstate.py  test_dirstate.py-20060728012006-d6mvoihjb3je9peu-2
    ------------------------------------------------------------
    revno: 2353.3.8.1.11
    merged: john at arbash-meinel.com-20070319204546-xxywya2iuemwqdt9
    parent: john at arbash-meinel.com-20070315233905-1keqs8pqeas1smq2
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Mon 2007-03-19 15:45:46 -0500
    message:
      Remove the unused _ignore_write_lock parameter.
    ------------------------------------------------------------
    revno: 2353.3.8.1.10
    merged: john at arbash-meinel.com-20070315233905-1keqs8pqeas1smq2
    parent: john at arbash-meinel.com-20070315232459-tf23624lgm7zz1yj
    parent: bialix at ukr.net-20070315232743-ol8q5vyqzpvdkrjy
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 18:39:05 -0500
    message:
      [merge] Alexander Belchenko: forgotten import of TestSkipped
        ------------------------------------------------------------
        revno: 2353.3.8.1.8.1.1
        merged: bialix at ukr.net-20070315232743-ol8q5vyqzpvdkrjy
        parent: john at arbash-meinel.com-20070315191520-3ogywmtl6v5gop7q
        committer: Alexander Belchenko <bialix at ukr.net>
        branch nick: jam-locking-bialix
        timestamp: Fri 2007-03-16 01:27:43 +0200
        message:
          forgotten import of TestSkipped
    ------------------------------------------------------------
    revno: 2353.3.8.1.9
    merged: john at arbash-meinel.com-20070315232459-tf23624lgm7zz1yj
    parent: john at arbash-meinel.com-20070315191520-3ogywmtl6v5gop7q
    parent: pqm at pqm.ubuntu.com-20070315231915-000d2bef502ae12b
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 18:24:59 -0500
    message:
      [merge] bzr.dev 2359
    ------------------------------------------------------------
    revno: 2353.3.8.1.8
    merged: john at arbash-meinel.com-20070315191520-3ogywmtl6v5gop7q
    parent: john at arbash-meinel.com-20070315185559-xtb5q6p8ldl2mvs1
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 13:15:20 -0600
    message:
      Use the right lock object.
    ------------------------------------------------------------
    revno: 2353.3.8.1.7
    merged: john at arbash-meinel.com-20070315185559-xtb5q6p8ldl2mvs1
    parent: john at arbash-meinel.com-20070315181445-8ig26uqlkq8bso3b
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 13:55:59 -0500
    message:
      Change the temporary_write_lock api, so that it always returns a lock object,
      and a flag indicating whether it was able to grab a write lock.
      This is because callers don't know whether it had to unlock the original lock, so it
      should always return something that lets callers maintain proper state.
    ------------------------------------------------------------
    revno: 2353.3.8.1.6
    merged: john at arbash-meinel.com-20070315181445-8ig26uqlkq8bso3b
    parent: john at arbash-meinel.com-20070315180216-jddyle9fxhr8tlde
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 13:14:45 -0500
    message:
      ctypes locks should return ctypes locks.
    ------------------------------------------------------------
    revno: 2353.3.8.1.5
    merged: john at arbash-meinel.com-20070315180216-jddyle9fxhr8tlde
    parent: john at arbash-meinel.com-20070315171948-5m28gvw153egui90
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 13:02:16 -0500
    message:
      Update DirState to use the new 'temporary_write_lock', and add tests that it works.
    ------------------------------------------------------------
    revno: 2353.3.8.1.4
    merged: john at arbash-meinel.com-20070315171948-5m28gvw153egui90
    parent: john at arbash-meinel.com-20070314234034-udg2sm249k9dqj8r
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Thu 2007-03-15 12:19:48 -0500
    message:
      Implement temporary_write_lock, and restore_read_lock for win32 locks.
    ------------------------------------------------------------
    revno: 2353.3.8.1.3
    merged: john at arbash-meinel.com-20070314234034-udg2sm249k9dqj8r
    parent: john at arbash-meinel.com-20070314223138-wfmodqhruo0g5l0e
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Wed 2007-03-14 18:40:34 -0500
    message:
      Implement a 'ReadLock.temporary_write_lock()' to upgrade to a write-lock in-process.
      Implement it for _fcntl.
      This is the api that can be used when we want to update a dirty dirstate
      even though we only have a read lock.
      On win32, this will actually unlock and re-lock the file. Which is part of
      the api description. But we don't have to limit ourselves to the lowest
      common denominator on all platforms.
    ------------------------------------------------------------
    revno: 2353.3.8.1.2
    merged: john at arbash-meinel.com-20070314223138-wfmodqhruo0g5l0e
    parent: john at arbash-meinel.com-20070313233914-bigtq3yxnmf90d9s
    parent: john at arbash-meinel.com-20070314205703-taih3n87g9n4afdv
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: locking
    timestamp: Wed 2007-03-14 17:31:38 -0500
    message:
      [merge] LockCleanup changes.
    ------------------------------------------------------------
    revno: 2353.3.8.1.1
    merged: 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.
-------------- next part --------------
=== added file 'bzrlib/tests/per_lock/test_temporary_write_lock.py'
--- a/bzrlib/tests/per_lock/test_temporary_write_lock.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/per_lock/test_temporary_write_lock.py	2007-03-15 18:55:59 +0000
@@ -0,0 +1,105 @@
+# Copyright (C) 2007 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""Tests for temporarily upgrading to a WriteLock."""
+
+from bzrlib import (
+    errors,
+    )
+
+from bzrlib.tests.per_lock import TestCaseWithLock
+
+
+class TestTemporaryWriteLock(TestCaseWithLock):
+
+    def setUp(self):
+        super(TestTemporaryWriteLock, self).setUp()
+        self.build_tree(['a-file'])
+
+    def test_can_upgrade_and_write(self):
+        """With only one lock, we should be able to write lock and switch back."""
+        a_lock = self.read_lock('a-file')
+        try:
+            success, t_write_lock = a_lock.temporary_write_lock()
+            self.assertTrue(success, "We failed to grab a write lock.")
+            try:
+                self.assertEqual('contents of a-file\n',
+                                 t_write_lock.f.read())
+                # We should be able to write to the file.
+                t_write_lock.f.seek(0)
+                t_write_lock.f.write('new contents for a-file\n')
+                t_write_lock.f.seek(0)
+                self.assertEqual('new contents for a-file\n',
+                                 t_write_lock.f.read())
+            finally:
+                a_lock = t_write_lock.restore_read_lock()
+        finally:
+            a_lock.unlock()
+
+    def test_is_write_locked(self):
+        """With a temporary write lock, we cannot grab another lock."""
+        a_lock = self.read_lock('a-file')
+        try:
+            success, t_write_lock = a_lock.temporary_write_lock()
+            self.assertTrue(success, "We failed to grab a write lock.")
+            try:
+                self.assertRaises(errors.LockContention,
+                                  self.write_lock, 'a-file')
+                self.assertRaises(errors.LockContention,
+                                  self.read_lock, 'a-file')
+            finally:
+                a_lock = t_write_lock.restore_read_lock()
+            # Now we only have a read lock, so we should be able to grab
+            # another read lock, but not a write lock
+            self.assertRaises(errors.LockContention,
+                              self.write_lock, 'a-file')
+            b_lock = self.read_lock('a-file')
+            b_lock.unlock()
+        finally:
+            a_lock.unlock()
+
+    def test_fails_when_locked(self):
+        """We can't upgrade to a write lock if something else locks."""
+        a_lock = self.read_lock('a-file')
+        try:
+            b_lock = self.read_lock('a-file')
+            try:
+                success, alt_lock = a_lock.temporary_write_lock()
+                self.assertFalse(success)
+                # Now, 'alt_lock' should be a read-lock on the file. It should
+                # either be the same object as a_lock, or a new object.
+                # If it is a new object, a_lock should be unlocked. (we don't
+                # want to end up with 2 locks on the file)
+                self.assertTrue((alt_lock is a_lock) or (a_lock.f is None))
+                # set a_lock = alt_lock so that cleanup works correctly
+                a_lock = alt_lock
+
+                # We should not be able to grab a write lock
+                # but we should be able to grab another read lock
+                self.assertRaises(errors.LockContention,
+                                  self.write_lock, 'a-file')
+                c_lock = self.read_lock('a-file')
+                c_lock.unlock()
+            finally:
+                b_lock.unlock()
+        finally:
+            a_lock.unlock()
+
+    # TODO: jam 20070314 to truly test these, we should be spawning an external
+    #       process, and having it lock/unlock/try lock on request.
+
+    # TODO: jam 20070314 Test that the write lock can fail if another process
+    #       holds a read lock. And that we recover properly.

=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2007-03-10 04:37:39 +0000
+++ b/bzrlib/dirstate.py	2007-03-15 19:15:20 +0000
@@ -1692,37 +1692,32 @@
         if (self._header_state == DirState.IN_MEMORY_MODIFIED or
             self._dirblock_state == DirState.IN_MEMORY_MODIFIED):
 
-            if self._lock_state == 'w':
-                out_file = self._state_file
-                wlock = None
-            else:
-                # Try to grab a write lock so that we can update the file.
-                try:
-                    wlock = lock.WriteLock(self._filename)
-                except (errors.LockError, errors.LockContention), e:
-                    # We couldn't grab the lock, so just leave things dirty in
-                    # memory.
+            grabbed_write_lock = False
+            if self._lock_state != 'w':
+                grabbed_write_lock, new_lock = self._lock_token.temporary_write_lock()
+                # Switch over to the new lock, as the old one may be closed.
+                # TODO: jam 20070315 We should validate the disk file has
+                #       not changed contents. Since temporary_write_lock may
+                #       not be an atomic operation.
+                self._lock_token = new_lock
+                self._state_file = new_lock.f
+                if not grabbed_write_lock:
+                    # We couldn't grab a write lock, so we switch back to a read one
                     return
-                except IOError, e:
-                    # This may be a read-only tree, or someone else may have a
-                    # ReadLock. so handle the case when we cannot grab a write
-                    # lock
-                    if e.errno in (errno.ENOENT, errno.EPERM, errno.EACCES,
-                                   errno.EAGAIN):
-                        # Ignore these errors and just don't save anything
-                        return
-                    raise
-                out_file = wlock.f
             try:
-                out_file.seek(0)
-                out_file.writelines(self.get_lines())
-                out_file.truncate()
-                out_file.flush()
+                self._state_file.seek(0)
+                self._state_file.writelines(self.get_lines())
+                self._state_file.truncate()
+                self._state_file.flush()
                 self._header_state = DirState.IN_MEMORY_UNMODIFIED
                 self._dirblock_state = DirState.IN_MEMORY_UNMODIFIED
             finally:
-                if wlock is not None:
-                    wlock.unlock()
+                if grabbed_write_lock:
+                    self._lock_token = self._lock_token.restore_read_lock()
+                    self._state_file = self._lock_token.f
+                    # TODO: jam 20070315 We should validate the disk file has
+                    #       not changed contents. Since restore_read_lock may
+                    #       not be an atomic operation.
 
     def _set_data(self, parent_ids, dirblocks):
         """Set the full dirstate data in memory.

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-03-15 15:12:45 +0000
+++ b/bzrlib/errors.py	2007-03-15 23:24:59 +0000
@@ -750,7 +750,7 @@
     # bits?
 
     internal_error = False
-    
+
     def __init__(self, lock):
         self.lock = lock
 

=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py	2007-03-15 23:25:18 +0000
+++ b/bzrlib/lock.py	2007-03-19 20:56:18 +0000
@@ -118,19 +118,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._open(filename, 'rb+')
-            if self.filename in self.open_locks:
+            # Check we can grab a lock before we actually open the file.
+            self.filename = osutils.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(self.filename, 'rb+')
             # 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.
@@ -144,16 +147,21 @@
                 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 = osutils.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
@@ -165,8 +173,84 @@
                 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()
 
+        def temporary_write_lock(self):
+            """Try to grab a write lock on the file.
+
+            On platforms that support it, this will upgrade to a write lock
+            without unlocking the file.
+            Otherwise, this will release the read lock, and try to acquire a
+            write lock.
+
+            :return: A token which can be used to switch back to a read lock.
+            """
+            assert self.filename not in _fcntl_WriteLock._open_locks
+            try:
+                wlock = _fcntl_TemporaryWriteLock(self)
+            except errors.LockError:
+                # We didn't unlock, so we can just return 'self'
+                return False, self
+            return True, wlock
+
+
+    class _fcntl_TemporaryWriteLock(_base_Lock):
+        """A token used when grabbing a temporary_write_lock.
+
+        Call restore_read_lock() when you are done with the write lock.
+        """
+
+        def __init__(self, read_lock):
+            super(_fcntl_TemporaryWriteLock, self).__init__()
+            self._read_lock = read_lock
+            self.filename = read_lock.filename
+
+            count = _fcntl_ReadLock._open_locks[self.filename]
+            if count > 1:
+                # Something else also has a read-lock, so we cannot grab a
+                # write lock.
+                raise errors.LockContention(self.filename)
+
+            assert self.filename not in _fcntl_WriteLock._open_locks
+
+            # See if we can open the file for writing. Another process might
+            # have a read lock. We don't use self._open() because we don't want
+            # to create the file if it exists. That would have already been
+            # done by _fcntl_ReadLock
+            try:
+                new_f = open(self.filename, 'rb+')
+            except IOError, e:
+                if e.errno in (errno.EACCES, errno.EPERM):
+                    raise errors.ReadOnlyLockError(self.filename, str(e))
+                raise
+            try:
+                # LOCK_NB will cause IOError to be raised if we can't grab a
+                # lock right away.
+                fcntl.lockf(new_f, fcntl.LOCK_SH | fcntl.LOCK_NB)
+            except IOError, e:
+                # TODO: Raise a more specific error based on the type of error
+                raise errors.LockError(e)
+            _fcntl_WriteLock._open_locks.add(self.filename)
+
+            self.f = new_f
+
+        def restore_read_lock(self):
+            """Restore the original ReadLock."""
+            # For fcntl, since we never released the read lock, just release the
+            # write lock, and return the original lock.
+            fcntl.lockf(self.f, fcntl.LOCK_UN)
+            self._clear_f()
+            _fcntl_WriteLock._open_locks.remove(self.filename)
+            # Avoid reference cycles
+            read_lock = self._read_lock
+            self._read_lock = None
+            return read_lock
+
 
     _lock_classes.append(('fcntl', _fcntl_WriteLock, _fcntl_ReadLock))
 
@@ -211,12 +295,38 @@
             super(_w32c_ReadLock, self).__init__()
             self._lock(filename, 'rb', LOCK_SH + LOCK_NB)
 
+        def temporary_write_lock(self):
+            """Try to grab a write lock on the file.
+
+            On platforms that support it, this will upgrade to a write lock
+            without unlocking the file.
+            Otherwise, this will release the read lock, and try to acquire a
+            write lock.
+
+            :return: A token which can be used to switch back to a read lock.
+            """
+            # I can't find a way to upgrade a read lock to a write lock without
+            # unlocking first. So here, we do just that.
+            self.unlock()
+            try:
+                wlock = _w32c_WriteLock(self.filename)
+            except errors.LockError:
+                return False, _w32c_ReadLock(self.filename)
+            return True, wlock
+
 
     class _w32c_WriteLock(_w32c_FileLock):
         def __init__(self, filename):
             super(_w32c_WriteLock, self).__init__()
             self._lock(filename, 'rb+', LOCK_EX + LOCK_NB)
 
+        def restore_read_lock(self):
+            """Restore the original ReadLock."""
+            # For win32 we had to completely let go of the original lock, so we
+            # just unlock and create a new read lock.
+            self.unlock()
+            return _w32c_ReadLock(self.filename)
+
 
     _lock_classes.append(('pywin32', _w32c_WriteLock, _w32c_ReadLock))
 
@@ -310,12 +420,37 @@
             super(_ctypes_ReadLock, self).__init__()
             self._lock(filename, 'rb', LOCK_SH + LOCK_NB)
 
+        def temporary_write_lock(self):
+            """Try to grab a write lock on the file.
+
+            On platforms that support it, this will upgrade to a write lock
+            without unlocking the file.
+            Otherwise, this will release the read lock, and try to acquire a
+            write lock.
+
+            :return: A token which can be used to switch back to a read lock.
+            """
+            # I can't find a way to upgrade a read lock to a write lock without
+            # unlocking first. So here, we do just that.
+            self.unlock()
+            try:
+                wlock = _ctypes_WriteLock(self.filename)
+            except errors.LockError:
+                return False, _ctypes_ReadLock(self.filename)
+            return True, wlock
 
     class _ctypes_WriteLock(_ctypes_FileLock):
         def __init__(self, filename):
             super(_ctypes_WriteLock, self).__init__()
             self._lock(filename, 'rb+', LOCK_EX + LOCK_NB)
 
+        def restore_read_lock(self):
+            """Restore the original ReadLock."""
+            # For win32 we had to completely let go of the original lock, so we
+            # just unlock and create a new read lock.
+            self.unlock()
+            return _ctypes_ReadLock(self.filename)
+
 
     _lock_classes.append(('ctypes', _ctypes_WriteLock, _ctypes_ReadLock))
 

=== modified file 'bzrlib/tests/per_lock/__init__.py'
--- a/bzrlib/tests/per_lock/__init__.py	2007-03-14 20:15:52 +0000
+++ b/bzrlib/tests/per_lock/__init__.py	2007-03-14 23:40:34 +0000
@@ -67,6 +67,7 @@
     result = tests.TestSuite()
     test_lock_implementations = [
         'bzrlib.tests.per_lock.test_lock',
+        'bzrlib.tests.per_lock.test_temporary_write_lock',
         ]
     adapter = LockTestProviderAdapter(lock._lock_classes)
     loader = tests.TestLoader()

=== modified file 'bzrlib/tests/per_lock/test_lock.py'
--- a/bzrlib/tests/per_lock/test_lock.py	2007-03-15 19:22:25 +0000
+++ b/bzrlib/tests/per_lock/test_lock.py	2007-03-15 23:24:59 +0000
@@ -99,14 +99,21 @@
         # Taking out a lock on a locked file should raise LockContention
         self.assertRaises(errors.LockContention, self.write_lock, '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 = self.read_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # Taking out a lock on a locked file should raise LockContention
         self.assertRaises(errors.LockContention, self.write_lock, '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 = self.read_lock('a-file')
+        a_lock.unlock()
+        a_lock = self.write_lock('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, self.read_lock, 'a-file')
+
+    def test_write_unlock_read(self):
+        """If we have removed the write lock, we can grab a read lock."""
+        a_lock = self.write_lock('a-file')
+        a_lock.unlock()
+        a_lock = self.read_lock('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 = self.read_lock('a-file')
+            b_lock = self.read_lock('a-file')
+            self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
+            a_lock.unlock()
+            a_lock = None
+            self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
+            b_lock.unlock()
+            b_lock = None
+            c_lock = self.write_lock('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()

=== modified file 'bzrlib/tests/test_commit_merge.py'
--- a/bzrlib/tests/test_commit_merge.py	2007-03-11 22:15:59 +0000
+++ b/bzrlib/tests/test_commit_merge.py	2007-03-15 23:27:43 +0000
@@ -18,7 +18,10 @@
 import os
 import shutil
 
-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import (
+    TestCaseWithTransport,
+    TestSkipped,
+    )
 from bzrlib.branch import Branch
 from bzrlib.errors import PointlessCommit, BzrError
 from bzrlib.tests.test_revision import make_branches

=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py	2007-03-13 02:12:11 +0000
+++ b/bzrlib/tests/test_dirstate.py	2007-03-15 23:24:59 +0000
@@ -367,6 +367,102 @@
         finally:
             state.unlock()
 
+    def test_can_save_in_read_lock(self):
+        self.build_tree(['a-file'])
+        state = dirstate.DirState.initialize('dirstate')
+        try:
+            # No stat and no sha1 sum.
+            state.add('a-file', 'a-file-id', 'file', None, '')
+            state.save()
+        finally:
+            state.unlock()
+
+        # Now open in readonly mode
+        state = dirstate.DirState.on_file('dirstate')
+        state.lock_read()
+        try:
+            entry = state._get_entry(0, path_utf8='a-file')
+            # The current sha1 sum should be empty
+            self.assertEqual('', entry[1][0][1])
+            # We should have a real entry.
+            self.assertNotEqual((None, None), entry)
+            sha1sum = state.update_entry(entry, 'a-file', os.lstat('a-file'))
+            # We should have gotten a real sha1
+            self.assertEqual('ecc5374e9ed82ad3ea3b4d452ea995a5fd3e70e3',
+                             sha1sum)
+
+            # The dirblock has been updated
+            self.assertEqual(sha1sum, entry[1][0][1])
+            self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
+                             state._dirblock_state)
+
+            del entry
+            # Now, since we are the only one holding a lock, we should be able
+            # to save and have it written to disk
+            state.save()
+        finally:
+            state.unlock()
+
+        # Re-open the file, and ensure that the state has been updated.
+        state = dirstate.DirState.on_file('dirstate')
+        state.lock_read()
+        try:
+            entry = state._get_entry(0, path_utf8='a-file')
+            self.assertEqual(sha1sum, entry[1][0][1])
+        finally:
+            state.unlock()
+
+    def test_save_fails_quietly_if_locked(self):
+        """If dirstate is locked, save will fail without complaining."""
+        self.build_tree(['a-file'])
+        state = dirstate.DirState.initialize('dirstate')
+        try:
+            # No stat and no sha1 sum.
+            state.add('a-file', 'a-file-id', 'file', None, '')
+            state.save()
+        finally:
+            state.unlock()
+
+        state = dirstate.DirState.on_file('dirstate')
+        state.lock_read()
+        try:
+            entry = state._get_entry(0, path_utf8='a-file')
+            sha1sum = state.update_entry(entry, 'a-file', os.lstat('a-file'))
+            # We should have gotten a real sha1
+            self.assertEqual('ecc5374e9ed82ad3ea3b4d452ea995a5fd3e70e3',
+                             sha1sum)
+            self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
+                             state._dirblock_state)
+
+            # Now, before we try to save, grab another dirstate, and take out a
+            # read lock.
+            # TODO: jam 20070315 Ideally this would be locked by another
+            #       process. To make sure the file is really OS locked.
+            state2 = dirstate.DirState.on_file('dirstate')
+            state2.lock_read()
+            try:
+                # This won't actually write anything, because it couldn't grab
+                # a write lock. But it shouldn't raise an error, either.
+                # TODO: jam 20070315 We should probably distinguish between
+                #       being dirty because of 'update_entry'. And dirty
+                #       because of real modification. So that save() *does*
+                #       raise a real error if it fails when we have real
+                #       modifications.
+                state.save()
+            finally:
+                state2.unlock()
+        finally:
+            state.unlock()
+        
+        # The file on disk should not be modified.
+        state = dirstate.DirState.on_file('dirstate')
+        state.lock_read()
+        try:
+            entry = state._get_entry(0, path_utf8='a-file')
+            self.assertEqual('', entry[1][0][1])
+        finally:
+            state.unlock()
+
 
 class TestDirStateInitialize(TestCaseWithDirState):
 



More information about the bazaar-commits mailing list