if a cleanup function for a lrucache raises, the cache is broken
Bug #396838 reported by
Michael Hudson-Doyle
This bug affects 1 person
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Bazaar |
Fix Released
|
Low
|
John A Meinel | ||
Bug Description
I was hunting thread-safety bugs, but ended up with this little snippet:
from bzrlib.lru_cache import LRUCache
def cleanup(*args):
1/0
c = LRUCache(10)
s = range(20)
c.add(1, 1, cleanup)
for k in s:
try:
c.add(k, 1)
except ZeroDivisionError:
print 'caught'
Related branches
lp:~jameinel/bzr/1.17-lru-cleanup
- Vincent Ladeuil: Needs Fixing
- Diff: 168 lines
Revision history for this message
| John A Meinel (jameinel) wrote | #1 |
| Changed in bzr: | |
| importance: | Undecided → Low |
| status: | New → Triaged |
| assignee: | nobody → John A Meinel (jameinel) |
| status: | Triaged → In Progress |
Revision history for this message
| John A Meinel (jameinel) wrote | #2 |
| Changed in bzr: | |
| status: | In Progress → Fix Committed |
Revision history for this message
| John A Meinel (jameinel) wrote | #3 |
| Changed in bzr: | |
| status: | Fix Committed → Fix Released |
| milestone: | none → 1.17 |
To post a comment you must log in.
cleanup code raising exceptions is generally considered improper anyway. Considering that the exception being raised is independent of the thing being added, it isn't really clear who should take responsibility.
1) I can agree that we should program defensively and try to maintain the linked list invariants even when cleanup raises an exception
2) Potentially we could trap and suppress all exceptions during cleanup, turning it into a warning (that is what python itself does during __del__ cleanups)
In the immediate term, we could just do:
try:
run_cleanup
finally:
update prev/next pointers