| msg16130 - (view) |
Author: Mike C. Fletcher (mcfletch) |
Date: 2003-05-24 20:24 |
| 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 |
|
|
| msg16131 - (view) |
Author: Tim Peters (tim.peters) *  |
Date: 2003-05-24 20:50 |
| Logged In: YES user_id=31435 Assigned to Fred (the original author, IIRC), and boosted priority a notch. |
|
|
| msg16132 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-05-25 00:36 |
| 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). |
|
|
| msg16133 - (view) |
Author: Mike C. Fletcher (mcfletch) |
Date: 2003-05-25 00:58 |
| 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. |
|
|
| msg16134 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-05-25 01:30 |
| 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. |
|
|
| msg16135 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-05-25 01:47 |
| 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. |
|
|
| msg16136 - (view) |
Author: Mike C. Fletcher (mcfletch) |
Date: 2003-05-27 17:43 |
| 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. |
|
|
| msg16137 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2003-05-29 13:06 |
| Logged In: YES user_id=12800 With Fred's approval, we backported this to 2.2.3 |
|
|