Rev 2364: Implement a 'ReadLock.temporary_write_lock()' to upgrade to a write-lock in-process. in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking

John Arbash Meinel john at arbash-meinel.com
Wed Mar 14 23:40:57 GMT 2007


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

------------------------------------------------------------
revno: 2364
revision-id: 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.
added:
  bzrlib/tests/per_lock/test_temporary_write_lock.py test_temporary_write-20070314233412-xp3ocbyvw3woa03w-1
modified:
  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
-------------- 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-14 23:40:34 +0000
@@ -0,0 +1,76 @@
+# 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:
+            t_write_lock = a_lock.temporary_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:
+            t_write_lock = a_lock.temporary_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()
+
+    # 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/errors.py'
--- a/bzrlib/errors.py	2007-03-14 20:47:17 +0000
+++ b/bzrlib/errors.py	2007-03-14 23:40:34 +0000
@@ -747,7 +747,7 @@
     # bits?
 
     internal_error = False
-    
+
     def __init__(self, lock):
         self.lock = lock
 

=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py	2007-03-14 22:31:38 +0000
+++ b/bzrlib/lock.py	2007-03-14 23:40:34 +0000
@@ -154,10 +154,10 @@
 
         _open_locks = {}
 
-        def __init__(self, filename):
+        def __init__(self, filename, _ignore_write_lock=False):
             super(_fcntl_ReadLock, self).__init__()
             self.filename = osutils.realpath(filename)
-            if self.filename in _fcntl_WriteLock._open_locks:
+            if not _ignore_write_lock and 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
@@ -179,6 +179,74 @@
                 _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
+            return _fcntl_TemporaryWriteLock(self)
+
+
+    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))
 

=== 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()



More information about the bazaar-commits mailing list