Issue33814
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.
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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:01 | admin | set | github: 77995 |
2018-06-10 00:46:08 | tim.peters | set | status: open -> closed nosy: + tim.peters messages: + msg319191 resolution: not a bug stage: resolved |
2018-06-10 00:25:02 | steven.daprano | set | messages: + msg319190 |
2018-06-10 00:07:36 | steven.daprano | set | nosy:
+ steven.daprano messages: + msg319188 |
2018-06-09 17:44:32 | eric.smith | set | nosy:
+ eric.smith messages: + msg319164 |
2018-06-09 16:50:20 | bup | create |