Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Generator cleanup without tp_del #62007

Closed
pitrou opened this issue Apr 21, 2013 · 9 comments
Closed

Generator cleanup without tp_del #62007

pitrou opened this issue Apr 21, 2013 · 9 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Apr 21, 2013

BPO 17807
Nosy @ncoghlan, @pitrou, @benjaminp, @phmc
Files
  • gen2.patch
  • gen3.patch
  • gen4.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2013-05-18.12:35:40.802>
created_at = <Date 2013-04-21.01:37:31.420>
labels = ['interpreter-core', 'type-feature']
title = 'Generator cleanup without tp_del'
updated_at = <Date 2013-05-18.12:35:40.801>
user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2013-05-18.12:35:40.801>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2013-05-18.12:35:40.802>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2013-04-21.01:37:31.420>
creator = 'pitrou'
dependencies = []
files = ['29960', '29962', '30175']
hgrepos = []
issue_num = 17807
keywords = ['patch']
message_count = 9.0
messages = ['187482', '187485', '187491', '188721', '188727', '188728', '189242', '189243', '189511']
nosy_count = 6.0
nosy_names = ['ncoghlan', 'pitrou', 'benjamin.peterson', 'python-dev', 'pconnell', 'isoschiz']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17807'
versions = ['Python 3.4']
    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 21, 2013

    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.

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Apr 21, 2013
    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 21, 2013

    Patch with added tests.

    @ncoghlan
    Copy link
    Contributor

    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.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 8, 2013

    Updated patch keeping PyGen_NeedsFinalizing() for backwards compatibility (always returning 0).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 8, 2013

    New changeset c89febab4648 by Antoine Pitrou in branch 'default':
    Issue bpo-17807: Generators can now be finalized even when they are part of a reference cycle.
    http://hg.python.org/cpython/rev/c89febab4648

    @pitrou
    Copy link
    Member Author

    pitrou commented May 8, 2013

    Committing for now, we'll see what people say.

    @pitrou pitrou closed this as completed May 8, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 14, 2013

    New changeset edefd3450834 by Antoine Pitrou in branch 'default':
    Backout c89febab4648 following private feedback by Guido.
    http://hg.python.org/cpython/rev/edefd3450834

    @pitrou
    Copy link
    Member Author

    pitrou commented May 14, 2013

    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.

    @pitrou pitrou reopened this May 14, 2013
    @pitrou
    Copy link
    Member Author

    pitrou commented May 18, 2013

    Obsoleted by PEP-442.

    @pitrou pitrou closed this as completed May 18, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @ncoghlan @pitrou