Rev 2513: (broken) removing lock counting from LockableFiles in http://sourcefrog.net/bzr/counted-lock

Martin Pool mbp at sourcefrog.net
Thu Jun 7 03:15:37 BST 2007


At http://sourcefrog.net/bzr/counted-lock

------------------------------------------------------------
revno: 2513
revision-id: mbp at sourcefrog.net-20070607021535-qaqik6lsl7mvu01j
parent: mbp at sourcefrog.net-20070606233823-m5t33u2altd078sx
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: counted-lock
timestamp: Thu 2007-06-07 12:15:35 +1000
message:
  (broken) removing lock counting from LockableFiles
modified:
  bzrlib/lockable_files.py       control_files.py-20051111201905-bb88546e799d669f
  bzrlib/tests/test_counted_lock.py test_counted_lock.py-20070502135927-7dk86io3ok7ctx6k-2
  bzrlib/workingtree_4.py        workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
=== modified file 'bzrlib/lockable_files.py'
--- a/bzrlib/lockable_files.py	2007-04-13 01:57:12 +0000
+++ b/bzrlib/lockable_files.py	2007-06-07 02:15:35 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006 Canonical Ltd
+# Copyright (C) 2005, 2006, 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
@@ -19,6 +19,7 @@
 #import traceback
 
 import bzrlib
+from bzrlib import counted_lock
 from bzrlib.decorators import (needs_read_lock,
         needs_write_lock)
 import bzrlib.errors as errors
@@ -44,9 +45,7 @@
     LockableFiles also provides some policy on top of Transport for encoding
     control files as utf-8.
 
-    LockableFiles manage a lock count and can be locked repeatedly by
-    a single caller.  (The underlying lock implementation generally does not
-    support this.)
+    LockableFiles use a CountedLock so this object's lock is reentrant.
 
     Instances of this class are often called control_files.
     
@@ -58,11 +57,6 @@
     OSLocks for the local filesystem.
     """
 
-    # _lock_mode: None, or 'r' or 'w'
-
-    # _lock_count: If _lock_mode is true, a positive count of the number of
-    # times the lock has been taken *by this process*.   
-    
     # If set to False (by a plugin, etc) BzrBranch will not set the
     # mode on created files or directories
     _set_file_mode = True
@@ -80,13 +74,12 @@
         self._transport = transport
         self.lock_name = lock_name
         self._transaction = None
-        self._lock_mode = None
-        self._lock_count = 0
         self._find_modes()
         esc_name = self._escape(lock_name)
         self._lock = lock_class(transport, esc_name,
                                 file_modebits=self._file_mode,
                                 dir_modebits=self._dir_mode)
+        self._counted_lock = counted_lock.CountedLock(self._lock)
 
     def create_lock(self):
         """Create the lock.
@@ -115,7 +108,7 @@
 
         The current ui factory will be used to prompt for user conformation.
         """
-        self._lock.break_lock()
+        self._counted_lock.break_lock()
 
     def _escape(self, file_or_path):
         if not isinstance(file_or_path, basestring):
@@ -245,59 +238,28 @@
         some other way, and need to synchronise this object's state with that
         fact.
         """
-        # mutter("lock write: %s (%s)", self, self._lock_count)
-        # TODO: Upgrade locking to support using a Transport,
-        # and potentially a remote locking protocol
-        if self._lock_mode:
-            if self._lock_mode != 'w' or not self.get_transaction().writeable():
+        if self._counted_lock._lock_count:
+            if not self.get_transaction().writeable():
                 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)
-            #traceback.print_stack()
-            self._lock_mode = 'w'
-            self._lock_count = 1
             self._set_transaction(transactions.WriteTransaction())
-            self._token_from_lock = token_from_lock
-            return token_from_lock
+        return self._counted_lock.lock_write(token=token)
 
     def lock_read(self):
-        # mutter("lock read: %s (%s)", self, self._lock_count)
-        if self._lock_mode:
-            assert self._lock_mode in ('r', 'w'), \
-                   "invalid lock mode %r" % self._lock_mode
-            self._lock_count += 1
-        else:
-            self._lock.lock_read()
-            #note('read locking %s', self)
-            #traceback.print_stack()
-            self._lock_mode = 'r'
-            self._lock_count = 1
+        if self._counted_lock._lock_count == 0:
             self._set_transaction(transactions.ReadOnlyTransaction())
             # 5K may be excessive, but hey, its a knob.
             self.get_transaction().set_cache_size(5000)
+        return self._counted_lock.lock_read()
                         
     def unlock(self):
-        # mutter("unlock: %s (%s)", self, self._lock_count)
-        if not self._lock_mode:
-            raise errors.LockNotHeld(self)
-        if self._lock_count > 1:
-            self._lock_count -= 1
-        else:
-            #note('unlocking %s', self)
-            #traceback.print_stack()
+        self._counted_lock.unlock()
+        if self._counted_lock._lock_count == 0:
             self._finish_transaction()
-            try:
-                self._lock.unlock()
-            finally:
-                self._lock_mode = self._lock_count = None
 
     def is_locked(self):
         """Return true if this LockableFiles group is locked"""
-        return self._lock_count >= 1
+        return self._counted_lock.is_locked()
 
     def get_physical_lock_status(self):
         """Return physical lock status.

=== modified file 'bzrlib/tests/test_counted_lock.py'
--- a/bzrlib/tests/test_counted_lock.py	2007-06-06 23:38:23 +0000
+++ b/bzrlib/tests/test_counted_lock.py	2007-06-07 02:15:35 +0000
@@ -189,6 +189,8 @@
         l = CountedLock(real_lock)
         token = l.lock_write()
         self.assertTrue(token is not None)
+        token2 = l.lock_write()
+        self.assertEqual(token, token2)
         # normally you could not reenter a held lockdir, but it can be done by
         # passing a correct token
         l2 = CountedLock(LockDir(t, 'lock'))

=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py	2007-06-01 13:42:02 +0000
+++ b/bzrlib/workingtree_4.py	2007-06-07 02:15:35 +0000
@@ -306,7 +306,7 @@
 
     def flush(self):
         """Write all cached data to disk."""
-        if self._control_files._lock_mode != 'w':
+        if self._control_files._counted_lock._lock_mode != 'w':
             raise errors.NotWriteLocked(self)
         self.current_dirstate().save()
         self._inventory = None
@@ -838,7 +838,7 @@
         return result
 
     def _must_be_locked(self):
-        if not self._control_files._lock_count:
+        if not self._control_files.is_locked():
             raise errors.ObjectNotLocked(self)
 
     def _new_tree(self):
@@ -1109,10 +1109,10 @@
 
     def unlock(self):
         """Unlock in format 4 trees needs to write the entire dirstate."""
-        if self._control_files._lock_count == 1:
+        if self._control_files._counted_lock._lock_count == 1:
             # eventually we should do signature checking during read locks for
             # dirstate updates.
-            if self._control_files._lock_mode == 'w':
+            if self._control_files._counted_lock._lock_mode == 'w':
                 if self._dirty:
                     self.flush()
             if self._dirstate is not None:




More information about the bazaar-commits mailing list