[redirected to python-dev]
[Tim]
>> Someone review this, please! Final releases are getting close, Fred
>> (the weakref guy) won't be around until Tuesday, and the pre-patch
>> code can indeed raise spurious RuntimeErrors in the presence of
>> threads or mutating comparison functions.
>>
>> See the bug report for my confusions: I can't see any reason for why
>> __delitem__ iterated over the keys.
[Raymond Hettinger]
> Until reading the note on threads, I didn't see the error and thought
> the original code was valid because it returned after the deletion
> instead of continuing to loop through iterkeys.
Note that one of the new tests I checked in provoked RuntimeError without
threads.
>> The new one-liner implementation is much faster, can't raise
>> RuntimeError, and should be> better-behaved in all respects wrt threads.
> Yes, that solves the OP's problem.
>> Bugfix candidate for 2.2.3 too, if someone else agrees with this
>> patch.
> The original code does its contortions to avoid raising a KeyError
> whenever the dictionary entry might have disappeared due to the
> ref count falling to zero and then a new, equal key was formed later.
Sorry, I can't picture what you're trying to say. Show some code? If in a
weak-keyed dict d I do
d[k1] = v
del k1 # last reference, so the dict mutation went away
del d[k2] # where k2 happens to be compare equal to what k1 was
then I claim that *should* raise KeyError, and pretty obviously so. Note
that the other new test I checked in showed that
del d[whatever]
never raised KeyError before; I can't see how that can be called a feature,
and if someone thinks it was they neglected to document it, or write a test
that failed when I changed the behavior <wink>.
> If the data disappeared, then, I think ref(key) will return None
No, ref(x) never returns None, regardless of what x may be. It may raise
TypeError if x is not of a weakly referencable type, and it may raise
MemoryError if we don't have enough memory left to construct a weakref, but
those are the only things that can go wrong.
w = ref(x) followed later by w() will return None, iff x has gone away in
the meantime -- maybe that's what you're thinking of.
> which is a bummer because that is then used (in your patch)
> as a lookup key.
If x and y are two weakly-referencable objects (not necessarily distinct)
that compare equal, then
ref(x) == ref(y)
and
hash(ref(x)) == hash(ref(y))
so long as both ref(x)() and ref(y)() don't return None (i.e., so long as x
and y are both still alive).
Soo when I map
del d[k1]
to
del d.data[ref(k1)]
it will succeed if and only if d.data has a key for a still-live object, and
that key compares equal to k1; else it will raise KeyError (or maybe
TypeError if k1 is a silly key to test in a weak-keyed dict, or MemoryError
if we run out of memory). That's what I believe it should do.
> The safest approach (until Fred re-appears) is to keep the original
> approach but use keys() instead of iterkeys(). Then, wrap the
> actual deletion in a try / except KeyError to handle a thread
> race to delete the same weakref object.
I'm not clear on what that means. By "delete the same weakref object", do
you mean that both threads try to do
del d[k]
with the same k and the same weak-valued dict d? If so, then I think one of
them *should* see KeyError, exactly the same as if they tried pulling this
trick with a regular dict.
> I'm sure there is a better way and will take another look tomorrow.
Thanks for trying, but I still don't get it. It would help if you could
show specific code that you believe worked correctly before but is broken
now. I added two new tests showing what I believe to be code that was
broken before but works now, and no changes to the existing tests were
needed.
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4