Created on 2012-02-29 14:55 by Yury.Selivanov, last changed 2012-03-01 15:39 by pitrou. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| weakset_len.patch | Yury.Selivanov, 2012-02-29 14:55 | patch for WeakSet.__len__ method | review | |
| weaksetlen.patch | pitrou, 2012-02-29 16:20 | review | ||
| weaksetlen2.patch | pitrou, 2012-02-29 16:41 | review | ||
| Messages (8) | |||
|---|---|---|---|
| msg154636 - (view) | Author: Yury Selivanov (Yury.Selivanov) * | Date: 2012-02-29 14:55 | |
WeakSet has a bug in its '__len__' method, where it performs iteration though 'self.data' without ensuring _IterationGuard.
This leads to some hard to catch errors with the following traceback:
builtins.RuntimeError: Set changed size during iteration
File /usr/lib64/python3.2/_weakrefset.py, line 66, in __len__
return sum(x() is not None for x in self.data)
File /usr/lib64/python3.2/_weakrefset.py, line 66, in <genexpr>
return sum(x() is not None for x in self.data)
|
|||
| msg154644 - (view) | Author: Antoine Pitrou (pitrou) *  |
Date: 2012-02-29 16:20 | |
I can't find a way to reproduce as a unit test. Here is a patch making __len__ O(1) as well (instead of O(n)). Could you try it? (note : AFAICT, a similar issue exists for weak dicts) |
|||
| msg154647 - (view) | Author: Antoine Pitrou (pitrou) * |
Date: 2012-02-29 16:41 | |
New patch addressing similar issue with weak dicts, and adding more tests. |
|||
| msg154648 - (view) | Author: Yury Selivanov (Yury.Selivanov) * | Date: 2012-02-29 16:42 | |
Yes, I couldn't reproduce it either. However it frequently occurs on our buildbot, so tomorrow I'll give you an update if this patch resolves the issue or not. In any way, your patch fixes the current implementation at least from the performance perspective. I think it should be applied to 'WeakValueDictionary.__len__' as well. Thanks! |
|||
| msg154689 - (view) | Author: Yury Selivanov (Yury.Selivanov) * | Date: 2012-03-01 12:40 | |
As expected, it seems that the patch fixes the issue. |
|||
| msg154699 - (view) | Author: Roundup Robot (python-dev) | Date: 2012-03-01 15:32 | |
New changeset 1cd0688ff004 by Antoine Pitrou in branch '3.2': Issue #14159: Fix the len() of weak containers (WeakSet, WeakKeyDictionary, WeakValueDictionary) to return a better approximation when some objects are dead or dying. http://hg.python.org/cpython/rev/1cd0688ff004 New changeset b1b2a29d3d81 by Antoine Pitrou in branch 'default': Issue #14159: Fix the len() of weak containers (WeakSet, WeakKeyDictionary, WeakValueDictionary) to return a better approximation when some objects are dead or dying. http://hg.python.org/cpython/rev/b1b2a29d3d81 |
|||
| msg154700 - (view) | Author: Roundup Robot (python-dev) | Date: 2012-03-01 15:38 | |
New changeset b6acfbe2bdbe by Antoine Pitrou in branch '2.7': Issue #14159: Fix the len() of weak sets to return a better approximation when some objects are dead or dying. http://hg.python.org/cpython/rev/b6acfbe2bdbe |
|||
| msg154701 - (view) | Author: Antoine Pitrou (pitrou) * |
Date: 2012-03-01 15:39 | |
2.7 didn't have the weak dict issue, but I still backported the tests there. Closing, should be fixed now. Thanks for reporting! |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2012-03-01 15:39:51 | pitrou | set | status: open -> closed resolution: fixed messages: + msg154701 stage: needs patch -> resolved |
| 2012-03-01 15:38:56 | python-dev | set | messages: + msg154700 |
| 2012-03-01 15:32:29 | python-dev | set | nosy:
+ python-dev messages: + msg154699 |
| 2012-03-01 12:40:21 | Yury.Selivanov | set | messages: + msg154689 |
| 2012-03-01 01:01:14 | rhettinger | set | priority: normal -> high |
| 2012-02-29 16:42:06 | Yury.Selivanov | set | messages: + msg154648 |
| 2012-02-29 16:41:57 | pitrou | set | files:
+ weaksetlen2.patch messages: + msg154647 |
| 2012-02-29 16:20:32 | pitrou | set | files:
+ weaksetlen.patch stage: needs patch messages: + msg154644 versions: + Python 2.7 |
| 2012-02-29 14:55:23 | Yury.Selivanov | create | |