[issue17468] Generator memory leak

[Vikas]

Vikas added the comment:

I see this comment :
>> When iteration over a queryset raised an exception, the result cache 
>> remained initialized with an empty list, so subsequent iterations returned 
>> an empty list instead of raising an exception>>

What if we catch the exceptions? Will that cause not to leak memory?

--
nosy: +Vikas

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

The issue17807 patch has now been committed and should solve this issue. I 
would like to encourage anyone to test this change, as it is quite a 
significant departure from the previous semantics:
http://mail.python.org/pipermail/python-dev/2013-May/126102.html

--
assignee: docs@python - 
components: +Interpreter Core -Documentation
resolution:  - duplicate
status: open - closed
superseder:  - Generator cleanup without tp_del

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Nick Coghlan]

Nick Coghlan added the comment:

I'll create a separate issue for the tp_del - __del__ question, since that's a 
language design decision that *does* need Guido's input, but doesn't relate to 
the broader question of generators, cycles and potential memory leaks.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Nick Coghlan]

Nick Coghlan added the comment:

Issue 17800 is anyone wants to weigh in on the tp_del - __del__ question (I 
ended up not adding Guido back to that one either, since the original design 
was based on an assumption that's now demonstrably false, so it makes sense to 
update the behaviour)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Anssi Kääriäinen]

Anssi Kääriäinen added the comment:

I wonder if it would be better to reword the garbage collection docs to mention 
that Python can't collect objects if they are part of a reference cycle, and 
some of the objects in the reference cycle need to run code at gc time. Then 
mention that such objects include objects with __del__ and also generators if 
they do have some other blocks than loops in them (for example try or with 
blocks).

To me it seems this issue could be mitigated somewhat by collecting the objects 
if there is just one object with finalizer code in the cycle. It should be safe 
to run the single finalizer, then collect the whole cycle, right?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

 I wonder if it would be better to reword the garbage collection docs
 to mention that Python can't collect objects if they are part of a
 reference cycle, and some of the objects in the reference cycle need
 to run code at gc time. Then mention that such objects include objects
 with __del__ and also generators if they do have some other blocks
 than loops in them (for example try or with blocks).
 
 To me it seems this issue could be mitigated somewhat by collecting
 the objects if there is just one object with finalizer code in the
 cycle. It should be safe to run the single finalizer, then collect the
 whole cycle, right?

This is tricky because it relies on the fact that the object with
finalizer will be collected before any objects the finalizer relies on.
For a generator, this means it has to be finalized before its frame
object is cleared (otherwise, the finally block can't execute
correctly).

However, the GC doesn't collect those objects directly. It calls
tp_clear on each of them, hoping that tp_clear will break the reference
cycle (and at the same time collect some of the objects in the cycle).
But it's difficult to influence which objects are collected first. 

In gcgen.py's case, the reference cycle is comprised of the MyObj
instance, the generator, and the generator's frame:

  `self` (MyObj) - generator - frame - `self` (MyObj)

So if `self` is collected first, it will collect the generator before
the frame, and the generator's finally block can execute fine.

But if the frame is collected first, it will clear itself and it will be
too late for the generator's finally block to execute.

And if the generator is collected first... well, it can't, as it doesn't
have a tp_clear slot; but if it had, that would clear the frame and make
the finally block uncallable. One might argue that a generator's
tp_clear should call the finally block *before* clearing the frame.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Anssi Kääriäinen]

Anssi Kääriäinen added the comment:

I was imagining that the collection should happen in two passes. First check 
for tp_dels and call them if safe (single tp_del in cycle allowed). Then free 
the memory. The first pass is already there, it just doesn't collect callable 
tp_dels.

If it would be possible to turn an object into inaccessible state after tp_del 
was called it would be possible to call multiple tp_dels in a cycle. If the del 
tries to use already deleted object you will get some sort of runtime exception 
indicating access of already collected object (another option is to allow 
access to object which has already had __del__ called, but that seems 
problematic).

Now, both of the above are likely way more complicated to implement than what I 
imagine. I have very little knowledge of the technical details involved.

If I understand correctly your idea was to do something similar to above, but 
only for generators.

The current situation is somewhat ugly. First, I can imagine people wishing to 
do try-finally or something with self.lock: in a generator. These will leak 
in circular reference cases. The generator case is surprising, so surprising 
that even experienced programmers will do such mistakes (see the django ticket 
for one example). Second, from application programmers perspective the fact 
that __del__ wasn't called is usually similar to having a silent failure in 
their code - for example this can result in leaking database connections or 
other resources, not to mention possible problems if one has a with 
self.lock: block in a generator.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

In a sense, doing something like with self.lock in a generator is already a 
leak. Even if there wasn't a cycle, collection could be arbitrarily delayed (in 
partincular on non-CPython VMs). I wonder if making generators context managers 
which call close() on exit would help.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

Those are two different issues:
- not calling the finalizer in a timely manner
- never calling the finalizer and creating a memory leak through
gc.garbage

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

I realize, but if people were responsible and closed their generators, the 
second one would be as much of a problem.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Nick Coghlan]

Nick Coghlan added the comment:

We can't make ordinary generators innately context managers, as it makes the 
error too hard to detect when you accidentally leave out @contextmanager when 
using a generator to write a custom one.

You can already use contextlib.closing to forcibly close them when appropriate, 
so providing a decorator to implicitly map __exit__ to close wouldn't really 
save much.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Nick Coghlan]

Nick Coghlan added the comment:

To get back to Anssi's original suggestion... I think Anssi's proposal to allow 
finalisation to be skipped for try/except/else is legitimate. It's only finally 
clauses that we try to guarantee will execute, there's no such promise implied 
for ordinary except clauses.

We *don't care* if the generator *would* have caught the thrown GeneratorExit, 
we only care about ensuring that finally blocks are executed (including those 
implied by with statements). So if there aren't any finally clauses or with 
statements in the block stack, we should be able to just let the generator and 
frame get collected (as Anssi suggested), without trying to allow execution to 
complete.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

 We *don't care* if the generator *would* have caught the thrown
 GeneratorExit, we only care about ensuring that finally blocks are
 executed (including those implied by with statements). So if there
 aren't any finally clauses or with statements in the block stack, we
 should be able to just let the generator and frame get collected (as
 Anssi suggested), without trying to allow execution to complete.

That's a good point.

I'm also contemplating that the generator close() could be done from the
*frame*'s tp_clear, which would sidestep the issue of collect order
entirely: the finally block doesn't actually need the generator to
execute, it only needs the frame and its locals.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

I've opened issue17807 for an alternative generator cleanup scheme which solves 
the present issue.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy: +benjamin.peterson, ncoghlan

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

Generators exit using GeneratorExit, which you could possible catch.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Anssi Kääriäinen]

Anssi Kääriäinen added the comment:

True, except GeneratorExit will run at garbage collection time and this will 
result in reference cycle problems. Checking if there is no except 
GeneratorExit clause might be too complicated.

I still think this is worth a brief note in the gc docs. The gc docs talk only 
about __del__ methods, and there isn't one defined for the example generator. 
The generator does have something technically equivalent, but I don't spot any 
actual __del__ methods in the reference loop.

A docs mention will not help much in avoiding this problem, but it will at 
least make the code behaviour equivalent to documentation. A mention in the gc 
docs would have helped me when trying to debug the leak in Django.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

A documentation patch sounds good to me.

--
assignee:  - docs@python
components: +Documentation
nosy: +docs@python
type: resource usage - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Nick Coghlan]

Nick Coghlan added the comment:

Adding Guido because this appears to be due to a longstanding difference 
between the handling of tp_del and most other slots

Specifically, the reason for the odd behaviour appears to be that generator 
objects define tp_del [1] (which is what the cyclic gc *really* looks for), but 
while defining __del__ in Python will populate tp_del in the type slots, 
defining tp_del directly in a builtin or extension type doesn't create an 
exposed __del__ at the Python level (there's no wrapper function identified in 
the slot definition).

So, at the very least, the fact that builtin and extension types can define 
tp_del without creating a visible __del__ method needs to be documented as a 
CPython implementation detail.

However, I'm not sure we actually have a good *reason* for tp_del failing to 
generate __del__ automatically - it's been that way since Guido implemented it 
[2], but Guido's comment in that commit message about not needing a wrapper 
because types written in C can't define tp_del is clearly no longer accurate.

[1] http://hg.python.org/cpython/file/2.7/Objects/genobject.c#l328
[2] http://hg.python.org/cpython/annotate/71dca7c76fa2/Objects/typeobject.c#3955

--
nosy: +gvanrossum
versions: +Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

With the advent of yield-based asynchronous programming, it is going to be 
problematic if a generator caught in a reference cycle can create a memory leak.

--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

I think the creation of __del__ wrappers for extension types is separate from 
this issue of generator finalization.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Benjamin Peterson added the comment:

Yes. Hopefully, the async framework using generators can properly can close() 
on them, though.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Antoine Pitrou]

Antoine Pitrou added the comment:

 Yes. Hopefully, the async framework using generators can properly can
 close() on them, though.

With yield from-based generators, you don't need a trampoline anymore,
so the close() call is now left to the application developers (if I'm
not mistaken).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Martin Morrison]

Changes by Martin Morrison m...@ensoft.co.uk:


--
nosy: +pconnell

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Guido van Rossum]

Guido van Rossum added the comment:

Unless I'm the only person on earth who can understand this I beg to be left 
out of this bug. I just don't have the bandwidth right now to look into it. If 
it's really about GC and __del__ and generators, maybe Tim Peters would be 
willing to look into it? Or perhaps Neil Schemenauer, who IIRC did a lot of the 
early work on GC.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Benjamin Peterson]

Changes by Benjamin Peterson benja...@python.org:


--
nosy:  -gvanrossum

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Martin Morrison]

Changes by Martin Morrison m...@ensoft.co.uk:


--
nosy: +isoschiz

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc added the comment:

Well, there is a __del__: the one from the generator function. And it's part of 
the cycle.

--
nosy: +amaury.forgeotdarc

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Anssi Kääriäinen]

Anssi Kääriäinen added the comment:

I am trying to read the code, and it seems objects of type generator are 
uncollectable if the code of the generator has a block of other type than 
SETUP_LOOP. I can see how try-finally for example in the generator would cause 
problems - the finally might reference objects which are already collected. But 
plain try-except? What is the problem with that? (Not saying there isn't one, I 
am new to the code...).

I tried to change the code to also allow collecting generators with plain 
try-except (add in a check for SETUP_EXCEPTION in addition to SETUP_LOOP in 
PyGen_NeedsFinalizing()). This resolves the leak, and all tests pass. I can't 
tell if it is actually safe to return 0 in SETUP_EXCEPTION case.

After the change try-finally will still leak but that seems correct.

Maybe this limitation should be documented? I think I understand now why 
finally or with statement inside a cyclic referenced generator will end up in 
gc.garbage. However the docs tell that only objects with __del__ defined will 
leak. From user perspective generator doesn't have __del__, at least it is not 
to be found with pdb. Generators have potentially unsafe finalizer code but 
that isn't technically equivalent to having __del__ method defined as far as I 
can tell.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17468] Generator memory leak

[Anssi Kääriäinen]

New submission from Anssi Kääriäinen:

A generator is leaked to gc.garbage in a situation where `__del__` isn't 
defined. See the attached file for as-minimalistic test case as I could make. 
Tested on Python 3.3.0, 3.2.3 and 2.7.3.

Note that if the try-except is removed from iterator(), then there is no leak.

This is related to Django bug #19895 
(https://code.djangoproject.com/ticket/19895).

--
files: test.py
messages: 184515
nosy: Anssi.Kääriäinen
priority: normal
severity: normal
status: open
title: Generator memory leak
type: resource usage
versions: Python 2.7, Python 3.3
Added file: http://bugs.python.org/file29455/test.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17468
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com