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.

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!

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.

> 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)

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,

> 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.

I vote for option 1. We already eschew hacks with locals().

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)

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.

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.