Rev 4517: Fix bug #396838, Update LRUCache to maintain invariant even in http://bazaar.launchpad.net/~jameinel/bzr/1.17-lru-cleanup
John Arbash Meinel
john at arbash-meinel.com
Wed Jul 8 15:28:08 BST 2009
At http://bazaar.launchpad.net/~jameinel/bzr/1.17-lru-cleanup
------------------------------------------------------------
revno: 4517
revision-id: john at arbash-meinel.com-20090708142804-i9rkpi9dmnu7v3x1
parent: pqm at pqm.ubuntu.com-20090708062954-l77om4r4z10qvfos
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 1.17-lru-cleanup
timestamp: Wed 2009-07-08 09:28:04 -0500
message:
Fix bug #396838, Update LRUCache to maintain invariant even
if a cleanup function raises an exception.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2009-07-06 06:46:30 +0000
+++ b/NEWS 2009-07-08 14:28:04 +0000
@@ -79,12 +79,8 @@
transforms.
(Craig Hewetson, Martin Pool, #218206)
-* The ``log+`` decorator, useful in debugging or profiling, could cause
- "AttributeError: 'list' object has no attribute 'next'". This is now
- fixed. The log decorator no longer shows the elapsed time or transfer
- rate because they're available in the log prefixes and the transport
- activity display respectively.
- (Martin Pool, #340347)
+* ``LRUCache`` will maintain the linked list pointers even if a nodes
+ cleanup function raises an exception. (John Arbash Meinel, #396838)
* Progress bars are now suppressed again when the environment variable
``BZR_PROGRESS_BAR`` is set to ``none``.
@@ -107,6 +103,13 @@
removing the stacking location inside ``.bzr/branch``.
(Robert Collins, #376243)
+* The ``log+`` decorator, useful in debugging or profiling, could cause
+ "AttributeError: 'list' object has no attribute 'next'". This is now
+ fixed. The log decorator no longer shows the elapsed time or transfer
+ rate because they're available in the log prefixes and the transport
+ activity display respectively.
+ (Martin Pool, #340347)
+
* Unshelve works correctly when multiple zero-length files are present on
the shelf. (Aaron Bentley, #363444)
=== modified file 'bzrlib/lru_cache.py'
--- a/bzrlib/lru_cache.py 2009-05-01 20:20:37 +0000
+++ b/bzrlib/lru_cache.py 2009-07-08 14:28:04 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2008 Canonical Ltd
+# Copyright (C) 2006, 2008, 2009 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
@@ -48,11 +48,13 @@
self.next_key, prev_key)
def run_cleanup(self):
- if self.cleanup is not None:
- self.cleanup(self.key, self.value)
- self.cleanup = None
- # Just make sure to break any refcycles, etc
- self.value = None
+ try:
+ if self.cleanup is not None:
+ self.cleanup(self.key, self.value)
+ finally:
+ self.cleanup = None
+ # Just make sure to break any refcycles, etc
+ self.value = None
class LRUCache(object):
@@ -156,13 +158,16 @@
raise ValueError('cannot use _null_key as a key')
if key in self._cache:
node = self._cache[key]
- node.run_cleanup()
- node.value = value
- node.cleanup = cleanup
+ try:
+ node.run_cleanup()
+ finally:
+ node.value = value
+ node.cleanup = cleanup
+ self._record_access(node)
else:
node = _LRUNode(key, value, cleanup=cleanup)
self._cache[key] = node
- self._record_access(node)
+ self._record_access(node)
if len(self._cache) > self._max_cache:
# Trigger the cleanup
@@ -241,16 +246,18 @@
# If we have removed all entries, remove the head pointer as well
if self._least_recently_used is None:
self._most_recently_used = None
- node.run_cleanup()
- # Now remove this node from the linked list
- if node.prev is not None:
- node.prev.next_key = node.next_key
- if node.next_key is not _null_key:
- node_next = self._cache[node.next_key]
- node_next.prev = node.prev
- # And remove this node's pointers
- node.prev = None
- node.next_key = _null_key
+ try:
+ node.run_cleanup()
+ finally:
+ # Now remove this node from the linked list
+ if node.prev is not None:
+ node.prev.next_key = node.next_key
+ if node.next_key is not _null_key:
+ node_next = self._cache[node.next_key]
+ node_next.prev = node.prev
+ # And remove this node's pointers
+ node.prev = None
+ node.next_key = _null_key
def _remove_lru(self):
"""Remove one entry from the lru, and handle consequences.
=== modified file 'bzrlib/tests/test_lru_cache.py'
--- a/bzrlib/tests/test_lru_cache.py 2009-04-16 22:06:25 +0000
+++ b/bzrlib/tests/test_lru_cache.py 2009-07-08 14:28:04 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2008 Canonical Ltd
+# Copyright (C) 2006, 2008, 2009 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
@@ -132,6 +132,43 @@
cache[2] = 26
self.assertEqual([(2, 20), (2, 25)], cleanup_called)
+ def test_cleanup_error_maintains_linked_list(self):
+ cleanup_called = []
+ def cleanup_func(key, val):
+ cleanup_called.append((key, val))
+ raise ValueError('failure during cleanup')
+
+ cache = lru_cache.LRUCache(max_cache=10)
+ for i in xrange(10):
+ cache.add(i, i, cleanup=cleanup_func)
+ for i in xrange(10, 20):
+ self.assertRaises(ValueError,
+ cache.add, i, i, cleanup=cleanup_func)
+
+ self.assertEqual([(i, i) for i in xrange(10)], cleanup_called)
+
+ self.assertEqual(range(19, 9, -1), [n.key for n in cache._walk_lru()])
+
+ def test_cleanup_during_replace_still_replaces(self):
+ cleanup_called = []
+ def cleanup_func(key, val):
+ cleanup_called.append((key, val))
+ raise ValueError('failure during cleanup')
+
+ cache = lru_cache.LRUCache(max_cache=10)
+ for i in xrange(10):
+ cache.add(i, i, cleanup=cleanup_func)
+ self.assertRaises(ValueError,
+ cache.add, 1, 20, cleanup=cleanup_func)
+ # We also still update the recent access to this node
+ self.assertEqual([1, 9, 8, 7, 6, 5, 4, 3, 2, 0],
+ [n.key for n in cache._walk_lru()])
+ self.assertEqual(20, cache[1])
+
+ self.assertEqual([(1, 1)], cleanup_called)
+ self.assertEqual([1, 9, 8, 7, 6, 5, 4, 3, 2, 0],
+ [n.key for n in cache._walk_lru()])
+
def test_len(self):
cache = lru_cache.LRUCache(max_cache=10, after_cleanup_count=10)
More information about the bazaar-commits
mailing list