classification
Title: Fix reference leak in io.StringIO
Type: behavior Stage: resolved
Components: IO, Library (Lib) Versions: Python 3.1, Python 3.2
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, pitrou
Priority: normal Keywords: patch

Created on 2009-06-08 19:51 by alexandre.vassalotti, last changed 2009-10-24 12:21 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
fix_refleak_stringio.diff alexandre.vassalotti, 2009-06-08 19:51
fix_refleak_stringio-2.diff alexandre.vassalotti, 2009-06-09 05:17
fix_refleak_stringio-3.diff alexandre.vassalotti, 2009-06-09 20:08
Messages (7)
msg89105 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-06-08 19:51
io.StringIO does not clear its reference to its attributes dictionary
when deleted. This causes a leak when io.StringIO has attributes. 

>>> def leak():
...    for _ in range(100):
...      f = io.StringIO()
...      f.foo = 1
... 
[39348 refs]
>>> leak()
[39650 refs]
>>> leak()
[39950 refs]
>>> leak()
[40250 refs]
msg89107 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-06-08 20:10
It seems wrong to call PyObject_ClearWeakRefs() in stringio_clear().
Weakrefs should only be notified when the object is deallocated, not
cleared.
Besides, you should add a test for this, so that the leak can be spotted
with regrtest -R.
msg89133 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-06-09 05:17
Here's an updated patch. The new patch also cleans up tp_clear,
tp_traverse and tp_dealloc of io.BytesIO which used weakreflist incorrectly.

I also added support for test_memoryio for adding reference leak
regression tests. As you'll see, the support is a bit heavyweight for
the only test case there. So perhaps I could change this to something
simpler.
msg89143 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-06-09 11:09
Why do you need all this? Isn't it enough to take a weakref and check
the callback is triggered?
(besides, we should avoid tests which only work in debug mode)
msg89162 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-06-09 20:08
> Why do you need all this? Isn't it enough to take a weakref and check
> the callback is triggered?

No, because you would need a weak reference to the instance's __dict__,
which is unavailable for io.StringIO.

Anyway, here's a simplified patch without the fun experimental code. :-)
msg90790 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-07-22 02:26
Patch committed in r74155 (branches/py3k).
msg94410 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-24 12:21
I just noticed that this hadn't been committed to trunk. I did the
backport myself, but in the future please first commit IO changes to
trunk and then merge to py3k.
History
Date User Action Args
2009-10-24 12:21:44pitrousetmessages: + msg94410
2009-07-22 02:26:55alexandre.vassalottisetstatus: open -> closed
resolution: accepted
messages: + msg90790

stage: patch review -> resolved
2009-06-09 20:08:34alexandre.vassalottisetfiles: + fix_refleak_stringio-3.diff

messages: + msg89162
2009-06-09 11:09:44pitrousetmessages: + msg89143
2009-06-09 05:17:39alexandre.vassalottisetfiles: + fix_refleak_stringio-2.diff

messages: + msg89133
2009-06-08 20:10:44pitrousetnosy: + pitrou
messages: + msg89107
2009-06-08 19:51:35alexandre.vassalotticreate