classification
Title: frame.f_locals keeps references to things for too long
Type: behavior Stage:
Components: Interpreter Core Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, exarkun, facundobatista, gvanrossum, haypo, pitrou, rhettinger
Priority: normal Keywords:

Created on 2009-05-26 15:27 by exarkun, last changed 2011-05-07 00:03 by benjamin.peterson.

Messages (10)
msg88365 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-05-26 15:27
When a frame's f_locals attribute is accessed, one of two things happens:

  * If this is the first access, a new dictionary is created and it is
populated with the locals from the frame (since they are not stored
internally as a dictionary).  The dictionary is both saved and returned.
  * Otherwise, the previously saved dictionary is updated to reflect the
current locals of the frame and then returned.

This means that the lifetime of locals in the frame is strangely altered
if the frame's f_locals attribute is ever accessed.  In particular, the
locals are kept alive until the frame object is discarded or the
f_locals attribute is accessed again.

The drawback of this is that references to locals may be "leaked" in a
surprising and difficult to understand way.  Another is that when
debugging an application, this will cause objects to have a different
lifetime than they will when the application is run without the
debugger.  One way the former of these issues has manifest is described
in this Twisted bug report:

  http://twistedmatrix.com/trac/ticket/3853

There, I suggested three possible solutions, in the form of a
replacement API for the f_locals attribute which either:

    * Gave you a copy of the locals at the current time and didn't
pretend it would be automatically updated
    * Gave you a view onto the locals at the current time, instead of a
real dictionary, and didn't actually hold any references to the locals
itself, only granted access to the "canonical" locals data in the frame.
    * Gave you a real dictionary of the locals but didn't hold a strong
reference to that dictionary internally, so that if all external users
of the dictionary went away, the frame itself would also stop referring
to the locals.
msg88428 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-05-27 18:53
IMO, any of these three solutions should be accepted.
The second is of course the nicest but also the most complex one, and
it's probably not worth it.
I'm not sure the third solution is easily feasible given that dicts
currently don't accept weakref'ing. We could of course create a special
subclass of dict, but, again, is it worth it?
This leaves us with the first solution which is, IMO, good enough, and
obviously the simplest of all three. You're welcome to propose a patch!
msg88429 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-05-27 18:58
Oh, I forgot dictionaries aren't weakrefable.  That's such a pain, I
thought the third solution would be a good balance between easy and good. :/

Regarding the first solution, my only question right now is whether this
should be a new attribute/method, or if the behavior of f_locals should
just be changed.  The former would be nicer, I think, and perhaps
f_locals can be deprecated in favor of it.
msg88430 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-05-27 19:13
> Regarding the first solution, my only question right now is whether this
> should be a new attribute/method, or if the behavior of f_locals should
> just be changed.  The former would be nicer, I think, and perhaps
> f_locals can be deprecated in favor of it.

I think the behaviour of f_locals can just be changed.
(Other developers may disagree)
msg135210 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2011-05-05 14:45
Making a copy of f_locals values to return a dictionary that doesn't hold references to the locals of the frame is not that simple (for me, at least):

- shallow copy: we'll return always a new dict, with the values being a copy of locals; this is simpler, but what about other objects that are referenced in in those values? For example, in locals exists a list A which holds a reference to object B; the new dict we return will have copy of A, but this copy will still reference B.

- deep copy: we'll return a new dict with a deep copy of all values; this is safer, but what about memory? In any case, how we could do a deep copy here? calling Python's code copy.deepcopy()?

I want to add a fourth option to JP's initial ones:

- have other attribute, something like f_locals_weak that is a list of tuples [(name, value), ...], being the values a weak reference to the real locals.

What do you think?

Regards,
msg135214 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-05-05 15:50
> Making a copy of f_locals values to return a dictionary that doesn't 
> hold references to the locals of the frame is not that simple

I think you misunderstood the bug report. The issue here is not that locals are referenced from the dict, it's that the dict is kept alive (since it is cached) longer that it should.

By the way, reading the discussion again, I think we could simply make dicts weak-referenceable. A dict is already quite big in memory and I don't think adding a pointer to the struct would make a significant difference.
msg135216 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-05-05 16:15
I vote for option 1. We already eschew hacks with locals().
msg135229 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2011-05-05 18:45
Antoine, to see if I understood correctly... if we build the dict, and just return it but don't save it in the frame, this leak would be solved? (yes, it'd be slower because everytime it's asked, it'd need to be built again)
msg135327 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-05-06 16:58
Le jeudi 05 mai 2011 à 18:45 +0000, Facundo Batista a écrit :
> 
> Antoine, to see if I understood correctly... if we build the dict, and
> just return it but don't save it in the frame, this leak would be
> solved? (yes, it'd be slower because everytime it's asked, it'd need
> to be built again)

I would let Jean-Paul confirm the answer, but: yes.
msg135389 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-05-07 00:03
I tried making f_locals always a copy but it wreaks havoc with some code which expects changing it to "work", namely pdb. I resign myself to using weakrefs.
History
Date User Action Args
2011-05-07 00:03:23benjamin.petersonsetmessages: + msg135389
2011-05-06 16:58:18pitrousetmessages: + msg135327
2011-05-05 18:45:59facundobatistasetmessages: + msg135229
2011-05-05 16:15:19benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg135216
2011-05-05 15:50:09pitrousetnosy: + gvanrossum, rhettinger
messages: + msg135214
2011-05-05 14:45:58facundobatistasetnosy: + facundobatista
messages: + msg135210
2009-05-27 19:13:34pitrousetmessages: + msg88430
2009-05-27 18:58:07exarkunsetmessages: + msg88429
2009-05-27 18:53:55pitrousetnosy: + pitrou
messages: + msg88428
2009-05-26 18:28:22hayposetnosy: + haypo
2009-05-26 15:27:04exarkuncreate