homepage

You can't use iterators on wekref dicts because items
might be removed from the dictionaries while iterating
due to GC.
I've attached a script that illustrates the bug with
Python 2.3.2. It doesn't matter whether you use weak
key or weak value dicts.
If this can't be fixed, then the iteration methods should
either be removed or made to (lamely) create intermediate
lists to work around the problem.

I made a patch to fix the problem. The cleaning up of they weakref keys or 
values will be held until all references to iterators created by the 
weakdict are dead.
I also couldn't resist removing code duplication of code in items(), 
keys() and values().
At first, I couldn't understand why this whole remove(), _remove() and 
selfref() mechanism was in place. I had removed them and replaced them 
with methods, and the tests still passed. Then I realized it was to make 
sure keys and values didn't prevent the weak dicts from being freed. I 
added tests for this.

Patch has tests, may need updating.

Interesting patch. I think the intermediate assertEquals in
test_weak_*_dict_flushed_dead_items_when_iters_go_out are just testing
an implementation detail, only the final one should remain.
Also, it is likely the "code duplication" you are talking about was
there for performance reasons, so I'd suggest putting it back in.

About duplicated code and performance:
When I look at the duplicated code, I don't see anything that remotely 
looks like a performance tweak. Just to make sure, I made a bench:
#!/usr/bin/env python
import sys
sys.path.insert(0, 'Lib')
import timeit
import weakref
class Foo(object): pass
def setup():
 L = [Foo() for i in range(1000)]
 global d
 d = weakref.WeakValueDictionary(enumerate(L))
 del L[:500] # have some dead weakrefs
def do():
 d.values()
print timeit.timeit(do, setup, number=100000)
Results without the patch:
 ./python.exe weakref_bench.py
0.804216861725
Results with the patch:
$ ./python.exe weakref_bench.py
0.813000202179
I think the small difference in performance is more attributable to the 
extra processing the weakref dict does than the deduplication of the 
code itself.
About the test_weak_*_dict_flushed_dead_items_when_iters_go_out:
If a weakref dict keeps its weak reference alive, it's not an 
implementation detail, it's a bug. The whole point of using such dicts 
is to not keep keys or values alive when they go out.

Oh, that's me again not correctly reading my own tests. It's the 
*_are_not_held_* tests that test that no reference is kept.
I agree about the *_flushed_dead_items_* being an implementation detail.

> Results without the patch:
> ./python.exe weakref_bench.py
> 0.804216861725
> 
> Results with the patch:
> $ ./python.exe weakref_bench.py
> 0.813000202179
Thanks for the numbers, I see my worries were unfounded.
> About the test_weak_*_dict_flushed_dead_items_when_iters_go_out:
> 
> If a weakref dict keeps its weak reference alive, it's not an 
> implementation detail, it's a bug. The whole point of using such dicts 
> is to not keep keys or values alive when they go out.
I was talking about doing `self.assertEqual(len(d), self.COUNT)` before
deleting the iterators.

I can confirm that the patch applies with minimal fuzz to the
release26-maint branches and the trunk, and that the added tests fail
without the updated implementation in both cases.
Furthermore, Jim's original demo script emits it error with my stock 2.6.5
Python, but is silent with the patched trunk / 2.6 branch.

Probably old news, but this also affects 2.5.4.

If this is to go forward the patch will need porting to 2.7, 3.1 and 3.2

It looks like this issue has been fixed in issue7105 already. Can we close this ticket?

It's not yet fixed in 2.7 or 2.6. Updating versions.

We might as well backport Antoine's patch rather than take this one (even if mine for 2.x already). It would be weird to have 2 wildly different patches to solve the same problem.
Maybe close this ticket and flag issue7105 for backporting?

Agreed.