classification
Title: Generator cleanup without tp_del
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, isoschiz, ncoghlan, pconnell, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-04-21 01:37 by pitrou, last changed 2013-05-18 12:35 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
gen2.patch pitrou, 2013-04-21 01:40 review
gen3.patch pitrou, 2013-04-21 02:30 review
gen4.patch pitrou, 2013-05-08 13:30 review
Messages (9)
msg187482 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-21 01:37
This experimental patch proposes to defer generator cleanup to the frame itself. In this scheme, the generator frame's tp_clear throws the GeneratorExit if necessary, so as to call cleanup code. The generator doesn't have any tp_del anymore, as it is now impossible to resurrect a generator (the frame, though, can be resurrected; I have to add a test for that).

The net effect is that generators caught in a reference cycle can always be reclaimed, and their cleanup code is run in a valid frame.
msg187485 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-21 02:30
Patch with added tests.
msg187491 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-04-21 04:44
Just a couple of minor comments on review. Everything else I looked for (including the path where a generator failing to stop during frame deallocation leads to reporting an unraisable exception) seemed fine.

One aspect I find interesting is that we've had other patches which proposed reducing the level of poking around generators needed to do inside frame objects by moving (some of) that state to the generators. This patch goes the other way, by moving the related cleanup functionality into the frame objects.

I think that's actually a reasonable option - the frame is always going to be involved at some point in order to actually execute the cleanup code, so our only real chance to break the cycle is to eliminate the generator's involvement.
msg188721 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-08 13:30
Updated patch keeping PyGen_NeedsFinalizing() for backwards compatibility (always returning 0).
msg188727 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-08 16:12
New changeset c89febab4648 by Antoine Pitrou in branch 'default':
Issue #17807: Generators can now be finalized even when they are part of a reference cycle.
http://hg.python.org/cpython/rev/c89febab4648
msg188728 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-08 16:13
Committing for now, we'll see what people say.
msg189242 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-14 18:38
New changeset edefd3450834 by Antoine Pitrou in branch 'default':
Backout c89febab4648 following private feedback by Guido.
http://hg.python.org/cpython/rev/edefd3450834
msg189243 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-14 18:40
After getting some private feedback from Guido, I'm convinced this patch isn't ready for consumption. Objects visible from the frame when it is finalized can be in a weird, incomplete state (Guido reports failing to get the __class__ of an object, for instance).

I'm keeping the issue open for now and will try to find further ways to solve the underlying problem.
msg189511 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-18 12:35
Obsoleted by PEP 442.
History
Date User Action Args
2013-05-21 16:17:00pitrouunlinkissue17934 dependencies
2013-05-18 12:35:40pitrousetstatus: open -> closed
resolution: rejected
messages: + msg189511
2013-05-14 18:40:58pitrousetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg189243

stage: resolved ->
2013-05-14 18:38:02python-devsetmessages: + msg189242
2013-05-10 17:39:50pitroulinkissue17468 superseder
2013-05-08 16:13:23pitrousetstatus: open -> closed
resolution: fixed
messages: + msg188728

stage: patch review -> resolved
2013-05-08 16:12:46python-devsetnosy: + python-dev
messages: + msg188727
2013-05-08 13:30:42pitrousetfiles: + gen4.patch

messages: + msg188721
2013-05-08 11:54:18pitroulinkissue17934 dependencies
2013-04-21 08:44:11pconnellsetnosy: + pitrou
2013-04-21 08:22:21pconnellsetnosy: + pconnell, isoschiz, - pitrou
2013-04-21 04:48:14ncoghlanlinkissue17800 superseder
2013-04-21 04:44:58ncoghlansetmessages: + msg187491
2013-04-21 02:30:02pitrousetfiles: + gen3.patch

messages: + msg187485
2013-04-21 01:40:48pitrousetfiles: + gen2.patch
2013-04-21 01:40:41pitrousetfiles: - gen2.patch
2013-04-21 01:37:31pitroucreate