Rev 4121: (Michael Hudson) Remove __del__ from LockableFiles. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Mar 12 02:18:10 GMT 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4121
revision-id: pqm at pqm.ubuntu.com-20090312021804-624908fcy28eisfn
parent: pqm at pqm.ubuntu.com-20090312012854-1alv7gohle5o1wi9
parent: michael.hudson at canonical.com-20090311214421-t0n9w8akm4k50ar6
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-03-12 02:18:04 +0000
message:
(Michael Hudson) Remove __del__ from LockableFiles.
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
------------------------------------------------------------
revno: 4041.1.5
revision-id: michael.hudson at canonical.com-20090311214421-t0n9w8akm4k50ar6
parent: michael.hudson at canonical.com-20090227023836-htoth0azybo7tvfe
committer: Michael Hudson <michael.hudson at canonical.com>
branch nick: no-LockableFiles.__del__
timestamp: Thu 2009-03-12 10:44:21 +1300
message:
review comments
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
------------------------------------------------------------
revno: 4041.1.4
revision-id: michael.hudson at canonical.com-20090227023836-htoth0azybo7tvfe
parent: michael.hudson at canonical.com-20090226222808-url1udsulbge2r28
committer: Michael Hudson <michael.hudson at canonical.com>
branch nick: no-LockableFiles.__del__
timestamp: Fri 2009-02-27 15:38:36 +1300
message:
this makes more sense
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
------------------------------------------------------------
revno: 4041.1.3
revision-id: michael.hudson at canonical.com-20090226222808-url1udsulbge2r28
parent: michael.hudson at canonical.com-20090226220744-2xrce7lcr4njs1kx
committer: Michael Hudson <michael.hudson at canonical.com>
branch nick: no-LockableFiles.__del__
timestamp: Fri 2009-02-27 11:28:08 +1300
message:
_this_ works
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
------------------------------------------------------------
revno: 4041.1.2
revision-id: michael.hudson at canonical.com-20090226220744-2xrce7lcr4njs1kx
parent: michael.hudson at canonical.com-20090226220344-ung77qhffqe5xkvo
committer: Michael Hudson <michael.hudson at canonical.com>
branch nick: no-LockableFiles.__del__
timestamp: Fri 2009-02-27 11:07:44 +1300
message:
ahem
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
------------------------------------------------------------
revno: 4041.1.1
revision-id: michael.hudson at canonical.com-20090226220344-ung77qhffqe5xkvo
parent: pqm at pqm.ubuntu.com-20090224214044-953waog3od1tvlfr
committer: Michael Hudson <michael.hudson at canonical.com>
branch nick: no-LockableFiles.__del__
timestamp: Fri 2009-02-27 11:03:44 +1300
message:
this is terrible but it works
modified:
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
=== modified file 'bzrlib/lockable_files.py'
--- a/bzrlib/lockable_files.py 2009-01-17 01:30:58 +0000
+++ b/bzrlib/lockable_files.py 2009-03-11 21:44:21 +0000
@@ -43,6 +43,23 @@
# somewhat redundant with what's done in LockDir; the main difference is that
# LockableFiles permits reentrancy.
+class _LockWarner(object):
+ """Hold a counter for a lock and warn if GCed while the count is >= 1.
+
+ This is separate from LockableFiles because putting a __del__ on
+ LockableFiles can result in uncollectable cycles.
+ """
+
+ def __init__(self, repr):
+ self.lock_count = 0
+ self.repr = repr
+
+ def __del__(self):
+ if self.lock_count >= 1:
+ # There should have been a try/finally to unlock this.
+ warnings.warn("%r was gc'd while locked" % self.repr)
+
+
class LockableFiles(object):
"""Object representing a set of related files locked within the same scope.
@@ -88,7 +105,7 @@
self.lock_name = lock_name
self._transaction = None
self._lock_mode = None
- self._lock_count = 0
+ self._lock_warner = _LockWarner(repr(self))
self._find_modes()
esc_name = self._escape(lock_name)
self._lock = lock_class(transport, esc_name,
@@ -109,12 +126,6 @@
def __str__(self):
return 'LockableFiles(%s, %s)' % (self.lock_name, self._transport.base)
- def __del__(self):
- if self.is_locked():
- # do not automatically unlock; there should have been a
- # try/finally to unlock this.
- warnings.warn("%r was gc'd while locked" % self)
-
def break_lock(self):
"""Break the lock of this lockable files group if it is held.
@@ -253,13 +264,13 @@
if self._lock_mode != 'w' or not self.get_transaction().writeable():
raise errors.ReadOnlyError(self)
self._lock.validate_token(token)
- self._lock_count += 1
+ self._lock_warner.lock_count += 1
return self._token_from_lock
else:
token_from_lock = self._lock.lock_write(token=token)
#traceback.print_stack()
self._lock_mode = 'w'
- self._lock_count = 1
+ self._lock_warner.lock_count = 1
self._set_transaction(transactions.WriteTransaction())
self._token_from_lock = token_from_lock
return token_from_lock
@@ -268,12 +279,12 @@
if self._lock_mode:
if self._lock_mode not in ('r', 'w'):
raise ValueError("invalid lock mode %r" % (self._lock_mode,))
- self._lock_count += 1
+ self._lock_warner.lock_count += 1
else:
self._lock.lock_read()
#traceback.print_stack()
self._lock_mode = 'r'
- self._lock_count = 1
+ self._lock_warner.lock_count = 1
self._set_transaction(transactions.ReadOnlyTransaction())
# 5K may be excessive, but hey, its a knob.
self.get_transaction().set_cache_size(5000)
@@ -281,19 +292,23 @@
def unlock(self):
if not self._lock_mode:
raise errors.LockNotHeld(self)
- if self._lock_count > 1:
- self._lock_count -= 1
+ if self._lock_warner.lock_count > 1:
+ self._lock_warner.lock_count -= 1
else:
#traceback.print_stack()
self._finish_transaction()
try:
self._lock.unlock()
finally:
- self._lock_mode = self._lock_count = None
+ self._lock_mode = self._lock_warner.lock_count = None
+
+ @property
+ def _lock_count(self):
+ return self._lock_warner.lock_count
def is_locked(self):
"""Return true if this LockableFiles group is locked"""
- return self._lock_count >= 1
+ return self._lock_warner.lock_count >= 1
def get_physical_lock_status(self):
"""Return physical lock status.
More information about the bazaar-commits
mailing list