[prev in list] [next in list] [prev in thread] [next in thread] 

List:       python-bugs-list
Subject:    [Python-bugs-list] [ python-Bugs-742860 ] WeakKeyDictionary __delitem__ uses iterkeys
From:       noreply () sourceforge ! net (SourceForge ! net)
Date:       2003-05-29 13:06:18
Message-ID: E19LN6o-0006Q7-00 () sc8-sf-web4 ! sourceforge ! net
[Download RAW message or body]

Bugs item #742860, was opened at 2003-05-24 16:24
Message generated for change (Settings changed) made by bwarsaw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=742860&group_id=5470

Category: Python Library
Group: Python 2.2.2
>Status: Closed
>Resolution: Fixed
Priority: 6
Submitted By: Mike C. Fletcher (mcfletch)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: WeakKeyDictionary __delitem__ uses iterkeys

Initial Comment:
iterkeys will raise a RuntimeError if the size of the
dictionary changes during iteration.  Deleting items
from the dictionary may cause cascade deletions which
will change the dictionary size.

Possible solutions:
    Use keys instead of iterkeys: line 155 of weakref.py:
        for ref in self.data.keys():

    Document the possibility that __delitem__ will
raise RuntimeErrors (not a preferable solution).

Note that there is also a potential race condition in
the __delitem__ method, where the key is del'd from the
data dictionary without a try: except: to catch cases
where the key is deleted between the time the key is
retrieved and the time the deletion occurs (which is
more likely if keys is used, but could still happen
with iterkeys).

Same problem is seen in both 2.2.2 and 2.2.3

----------------------------------------------------------------------

>Comment By: Barry A. Warsaw (bwarsaw)
Date: 2003-05-29 09:06

Message:
Logged In: YES 
user_id=12800

With Fred's approval, we backported this to 2.2.3

----------------------------------------------------------------------

Comment By: Mike C. Fletcher (mcfletch)
Date: 2003-05-27 13:43

Message:
Logged In: YES 
user_id=34901

Can't test 2.3 CVS just now (don't want to mess up the works
while I'm debugging a wxPython bug).  The one-liner:

    def __delitem__(self, key):
		del self.data[ref(key)]

does appear to work under Python 2.2.3.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-05-24 21:47

Message:
Logged In: YES 
user_id=31435

Mike, can you try current CVS Python (2.3)?  Fred won't be 
back for days, so I can't quiz him.  I implemented the one-
liner __delitem__, which is much faster, can't raise 
RuntimeError, and should be better behaved in all respects 
in the face of threads and/or comparison functions that 
mutate the dict as a side effect.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-05-24 21:30

Message:
Logged In: YES 
user_id=31435

Ah, threads.  OK, but if two threads both try to delete the 
same key, then one of them *should* see a KeyError -- 
same as for a regular dict.

----------------------------------------------------------------------

Comment By: Mike C. Fletcher (mcfletch)
Date: 2003-05-24 20:58

Message:
Logged In: YES 
user_id=34901

Re: race condition.  Thread 1 calls del d[x] and then thread
2 calls del d[x].  Both have strong refs to the key, so the
weakrefs don't die, both get the list of keys (weakrefs) to
scan before either deletes a key, both then try to delete
the item from the data dictionary.  That is:

t1:  del d[x]
t2:  del d[x]
t1:  gets keys()
t2:  gets keys()
t1:  finds key in keys, does del data[ref]
t2:  finds key in keys (even though it's no longer in the
dictionary), tries to do del data[ref], raises KeyError
because t1 has already removed the weakref key from the data
dictionary.

I may be wrong, of course.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-05-24 20:36

Message:
Logged In: YES 
user_id=31435

In staring at the code, I'm baffled as to why __delitem__ 
iterates over the keys at all.  Why isn't the implementation 
the one-liner

    del self.data[ref(key)]

?  That's basically what __contains__, has_key() and 
__getitem__ get away with.  Two refs to the same object 
have the same hash codes and compare equal, so I'm 
having a hard time seeing why that isn't good enough for 
__delitem__ too.

Mike, I didn't understand your point about the race 
condition.  The object passed to __delitem__ as the key 
has a strong reference to it merely by virtue of having been 
passed to __delitem__, so it can't go away until (at 
earliest) __delitem__ returns (and so drops its strong 
reference to key).

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-05-24 16:50

Message:
Logged In: YES 
user_id=31435

Assigned to Fred (the original author, IIRC), and boosted 
priority a notch.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=742860&group_id=5470


[prev in list] [next in list] [prev in thread] [next in thread]