classification
Title: __len__ method of weakset
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Yury.Selivanov, pitrou, python-dev, rhettinger
Priority: high Keywords: patch

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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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:51pitrousetstatus: open -> closed
resolution: fixed
messages: + msg154701

stage: needs patch -> resolved
2012-03-01 15:38:56python-devsetmessages: + msg154700
2012-03-01 15:32:29python-devsetnosy: + python-dev
messages: + msg154699
2012-03-01 12:40:21Yury.Selivanovsetmessages: + msg154689
2012-03-01 01:01:14rhettingersetpriority: normal -> high
2012-02-29 16:42:06Yury.Selivanovsetmessages: + msg154648
2012-02-29 16:41:57pitrousetfiles: + weaksetlen2.patch

messages: + msg154647
2012-02-29 16:20:32pitrousetfiles: + weaksetlen.patch

stage: needs patch
messages: + msg154644
versions: + Python 2.7
2012-02-29 14:55:23Yury.Selivanovcreate