This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: exec() maybe has a memory leak
Type: Stage: resolved
Components: Documentation, Interpreter Core Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: docs@python Nosy List: bup, docs@python, eric.smith, steven.daprano, tim.peters
Priority: normal Keywords:

Created on 2018-06-09 16:50 by bup, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Messages (5)
msg319157 - (view) Author: Dan Snider (bup) * Date: 2018-06-09 16:50
Sort of because I'm really not sure if it's working as intended or not. When I found this out today, I noticed that the documentation of `exec` has been updated since the last time I saw it, which is good, but it still leaves much to be desired. Specifically, I don't feel it does enough to explain the supposed justification for `exec`'s ability to very easily trigger a catastrophic memory leak someone unfamiliar with the code they're working on may never have the patience (if not time) to track down.. 

An illustration:

    import itertools
    import sys
    import weakref
    
    def noleak():
        
        def outer():
            from itertools import repeat as reeeee
            def f():
                return reeeee(1, 1)
            return f
        
        f = outer()
        c = sys.getrefcount(f)
        return c, weakref.ref(f), sys.getrefcount(itertools.repeat)

    def leaks():
        ns = {}
        co =  ('from itertools import repeat as reeeee\n'
               'def f(): return reeeee(1, 1)')
        pre = sys.getrefcount(ns)
        exec(co, ns)
        pos = sys.getrefcount(ns)
        return (pre, pos), weakref.ref(ns['f']), sys.getrefcount(itertools.repeat)

    for i in range(10):
        leaks()
    for i in range(10):
        noleak()

Perhaps I'm wrong in thinking that this is behaving as intended and it actually is a bug. Starting from builtin_exec_impl I got as far as _PyFrame_New_NoTrack before I threw up my hands and decided this is something for someone else. 

If `exec` with only a `globals` argument is indeed working as intended, I still think it's ridiculous that every item added to that dict has an "extra", non-obvious reference count that is impossible to vanquish of from within Python. I intuited leaving `locals` blank will do what usually happens when functions take optional arguments, and *usually* that is *not* the triggering of a "hidden" memory leak.


Perhaps the only solution here is to deprecate `exec` accepting `globals` and `locals` as optional arguments.
msg319164 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-06-09 17:44
It would be helpful if you could show what output you see, and how it differs from what you expect.

I think you're just seeing reference cycles or some other delayed garbage collection. If you put in a gc.collect() in your loops to force a collection, you'll see the reference numbers drop back.

There's no way we would deprecate globals and locals params to exec().
msg319188 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-06-10 00:07
Perhaps a little less self-righteous anger and a little more detail on this alleged bug would be appropriate.

Quote: 
I still think it's ridiculous that every item added to that dict has an "extra", non-obvious reference count that is impossible to vanquish of from within Python.

Have you demonstrated that this is the case? How do you know that is what is happening?

Quote:
I intuited leaving `locals` blank will do what usually happens when functions take optional arguments, and *usually* that is *not* the triggering of a "hidden" memory leak.

Have you demonstrated a memory leak? What is your intuition for what leaving locals blank should do? Did you try running the code without leaving locals blank?

Quote: 
the supposed justification for `exec`'s ability to very easily trigger a catastrophic memory leak

I wouldn't call your example code "very easily", nor do I know what "supposed justification" you are referring to. I read the docs for exec here

https://docs.python.org/3/library/functions.html#exec

and I see nothing justifying memory leaks or even mentioning them. Are you reading some other documentation?

Is this the shortest, simplest demonstration of the leak you can come up with? What output do you get and what output did you expect?

If it necessary for repeat to be imported under the name "reeeee" over and over again? What do weakrefs have to do with this?

You call this a "catastrophic" memory leak, I don't know whether this is mere hyperbole or something that will lock up or crash my computer when I run it.
msg319190 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-06-10 00:24
I decided to risk this "catastrophic" leak, and ran this:

py> x = leaks()
py> x
((2, 3), <weakref at 0xb7a1998c; to 'function' at 0xb789826c (f)>, 24)
py> import gc
py> gc.collect()
22
py> x
((2, 3), <weakref at 0xb7a1998c; dead>, 24)


so I think Eric is correct, it is just a garbage collection issue. Possibly a bug, possibly normal behaviour. I don't think it is a documentation bug, if it is a bug at all it might be a garbage collection bug.

But I really don't know, because I don't understand what Dan thinks is the memory leak here or why it happens. I *think* Dan might be referring to the fact that each time you call leaks(), there's one extra reference to itertools.repeat.

Dan, if you can confirm that's the leak you are referring to, we would have a better chance of deciding whether it is a bug and if so if it is specific to exec.
msg319191 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-06-10 00:46
Dan, your bug report is pretty much incoherent ;-)  This standard Stack Overflow advice applies here too:

https://stackoverflow.com/help/mcve

Guessing your complaint is that:

    sys.getrefcount(itertools.repeat)

keeps increasing by 1 across calls to `leaks()`, then as Eric said it will drop back to its original value after a `gc.collect()` call.

Steven, best I can tell weakrefs have nothing to do with this.  The refcount on itertools.repeat increases if

    weakref.ref(ns['f']), 

is deleted.

It's a simple reference cycle.  That is

    ns['f'].__globals__ is ns

is True, so `ns` can't be collected via reference counting, and so `ns` also holds a reference to the irrelevantly renamed `itertools.repeat` too until cyclic gc can detect the trash cycle.

Dan, if that's _not_ what you're complaining about, please follow the advice at the top and add a focused, minimal example of what you are talking about.  In the meantime, I'm closing this as not-a-bug.
History
Date User Action Args
2022-04-11 14:59:01adminsetgithub: 77995
2018-06-10 00:46:08tim.peterssetstatus: open -> closed

nosy: + tim.peters
messages: + msg319191

resolution: not a bug
stage: resolved
2018-06-10 00:25:02steven.dapranosetmessages: + msg319190
2018-06-10 00:07:36steven.dapranosetnosy: + steven.daprano
messages: + msg319188
2018-06-09 17:44:32eric.smithsetnosy: + eric.smith
messages: + msg319164
2018-06-09 16:50:20bupcreate