
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2013-05-08 11:54 bypitrou, last changed2022-04-11 14:57 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| frame_clear.patch | pitrou,2013-05-11 19:20 | review | ||
| frame_clear2.patch | pitrou,2013-05-11 20:04 | review | ||
| frame_clear_alt.patch | pitrou,2013-05-13 08:57 | review | ||
| frame_clear_alt2.patch | pitrou,2013-05-13 15:48 | review | ||
| frame_clear_alt3.patch | pitrou,2013-08-02 22:10 | review | ||
| Messages (14) | |||
|---|---|---|---|
| msg188718 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-08 11:54 | |
I think we may want to add a finalize() or close() method on frame objects which would clear all local variables (as well as dereference the globals dict, perhaps), after having optionally run a generator's close() method (if the frame belongs to a generator).If I'm not mistaken, it should allow breaking reference cycles, and remove the need for complex traceback processing, which Twisted currently also does:http://twistedmatrix.com/trac/browser/trunk/twisted/python/failure.py#L89Note that generator cleanup through the frame has a patch inissue17807.(spinned off fromissue17911) | |||
| msg188950 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-11 19:20 | |
Here is a patch. Guido, I would hope this may be useful to you. | |||
| msg188952 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-11 20:04 | |
Updated patch to avoid clearing executing frames.Note that the new f_executing member should be re-usable to compute gi_running without storing it. | |||
| msg188974 -(view) | Author: Alyssa Coghlan (ncoghlan)*![]() | Date: 2013-05-12 04:31 | |
Mostly looks good to me, but I think I'd prefer that attempts to clear a running frame raise RuntimeError with an appropriate message.I also wonder how this might relate to Eric Snow's proposal to reference the currently executing function from the frame object (seeissue 12857). It seems to me that the "f_func" pointer in that patch could serve the same purpose as the "f_executing" boolean flag in this patch, while providing additional information about the execution context.Some other possibly relevant traceback related resource management issues:issue 6116,issue 1565525,issue 9815 (picked up while searching for Eric's RFE above)(We may want to add a "clear_frames" convenience method to tracebacks as well) | |||
| msg189016 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-12 11:11 | |
> Mostly looks good to me, but I think I'd prefer that attempts to clear> a running frame raise RuntimeError with an appropriate message.Hmm, why not. My intuition was to make frame.clear() a best-effortmethod, but this sounds ok too.> I also wonder how this might relate to Eric Snow's proposal to> reference the currently executing function from the frame object (see>issue 12857). It seems to me that the "f_func" pointer in that patch> could serve the same purpose as the "f_executing" boolean flag in this> patch, while providing additional information about the execution> context.Yes, perhaps. Then Eric's patch can incorporate that change once theframe.clear() patch is committed.> (We may want to add a "clear_frames" convenience method to tracebacks> as well)That, or in the traceback module. The reason I'm proposing this one as aframe method is that it can't be done in pure Python. | |||
| msg189026 -(view) | Author: Alyssa Coghlan (ncoghlan)*![]() | Date: 2013-05-12 12:54 | |
+1 to all Antoine's replies :) | |||
| msg189109 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-13 08:57 | |
Here is an alternative patch raising RuntimeError on executing frames.Please tell which one you prefer :) | |||
| msg189149 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-13 15:48 | |
Updated patch addressing some of Nick's comments. | |||
| msg189151 -(view) | Author: Guido van Rossum (gvanrossum)*![]() | Date: 2013-05-13 17:07 | |
A downside of using this is that some extended traceback printers (such as cgitb.py in the stdlib, or and some things I've seen in web frameworks) won't be able to print the locals. And pdb's pm() function would lose its value too. So it'll remain a judgment call whether to use this.As for Tulip, I think I've broken the cycle I was most concerned about, and I've also gotten rid of the lambda that cost me so much time debugging. :-) | |||
| msg189768 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-05-21 16:17 | |
I will probably wait forPEP 442 adoption before finalizing this issue. | |||
| msg189975 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2013-05-25 16:00 | |
I may be safer to initialize f->f_executing to 1 at the creation of the frame, and only change its value to 0 when the execution is done. (The name of the attribute may also be changed.) I don't know if it is possible, but a frame should not be cleared just before it is used to execute code. | |||
| msg194214 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-08-02 22:10 | |
Here is an updated patch. Any objections to committing? | |||
| msg194511 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2013-08-05 21:28 | |
New changeset862ab99ab570 by Antoine Pitrou in branch 'default':Issue#17934: Add a clear() method to frame objects, to help clean up expensive details (local variables) and break reference cycles.http://hg.python.org/cpython/rev/862ab99ab570 | |||
| msg194512 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2013-08-05 21:32 | |
Ok, I committed the RuntimeError-raising version. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:45 | admin | set | github: 62134 |
| 2013-08-05 21:32:19 | pitrou | set | status: open -> closed resolution: fixed messages: +msg194512 stage: patch review -> resolved |
| 2013-08-05 21:28:01 | python-dev | set | nosy: +python-dev messages: +msg194511 |
| 2013-08-02 22:10:04 | pitrou | set | files: +frame_clear_alt3.patch messages: +msg194214 |
| 2013-06-18 19:04:48 | pitrou | set | dependencies: +PEP 442 implementation |
| 2013-05-25 16:00:54 | vstinner | set | nosy: +vstinner messages: +msg189975 |
| 2013-05-21 16:17:00 | pitrou | set | dependencies: -Generator cleanup without tp_del messages: +msg189768 |
| 2013-05-13 17:07:35 | gvanrossum | set | messages: +msg189151 |
| 2013-05-13 15:48:59 | pitrou | set | files: +frame_clear_alt2.patch messages: +msg189149 |
| 2013-05-13 08:57:07 | pitrou | set | files: +frame_clear_alt.patch messages: +msg189109 |
| 2013-05-12 12:54:03 | ncoghlan | set | messages: +msg189026 |
| 2013-05-12 11:11:20 | pitrou | set | messages: +msg189016 |
| 2013-05-12 08:14:45 | flox | set | nosy: +flox |
| 2013-05-12 04:31:50 | ncoghlan | set | nosy: +eric.snow messages: +msg188974 |
| 2013-05-11 20:04:20 | pitrou | set | files: +frame_clear2.patch messages: +msg188952 |
| 2013-05-11 19:20:54 | pitrou | set | files: +frame_clear.patch nosy: +gvanrossum messages: +msg188950 keywords: +patch stage: needs patch -> patch review |
| 2013-05-08 11:54:18 | pitrou | set | dependencies: +Generator cleanup without tp_del |
| 2013-05-08 11:54:02 | pitrou | create | |