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

Add gc.needs_finalizing() to check if an object needs finalising #62000

Closed
ncoghlan opened this issue Apr 20, 2013 · 8 comments
Closed

Add gc.needs_finalizing() to check if an object needs finalising #62000

ncoghlan opened this issue Apr 20, 2013 · 8 comments
Labels
easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

BPO 17800
Nosy @warsaw, @ncoghlan, @pitrou, @benjaminp, @alex
Superseder
  • bpo-17807: Generator cleanup without tp_del
  • 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 2015-04-14.23:59:24.906>
    created_at = <Date 2013-04-20.06:38:21.002>
    labels = ['interpreter-core', 'easy', 'type-feature']
    title = 'Add gc.needs_finalizing() to check if an object needs finalising'
    updated_at = <Date 2015-04-14.23:59:24.905>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2015-04-14.23:59:24.905>
    actor = 'benjamin.peterson'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-04-14.23:59:24.906>
    closer = 'benjamin.peterson'
    components = ['Interpreter Core']
    creation = <Date 2013-04-20.06:38:21.002>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 17800
    keywords = ['easy']
    message_count = 8.0
    messages = ['187409', '187412', '187417', '187428', '187435', '187436', '187438', '187493']
    nosy_count = 8.0
    nosy_names = ['barry', 'ncoghlan', 'pitrou', 'benjamin.peterson', 'alex', 'fijall', 'sbt', 'isoschiz']
    pr_nums = []
    priority = 'low'
    resolution = 'out of date'
    stage = 'needs patch'
    status = 'closed'
    superseder = '17807'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17800'
    versions = ['Python 3.4']

    @ncoghlan
    Copy link
    Contributor Author

    This came up in bpo-17468: currently, populating tp_del from C (as generators now do) doesn't automatically create a __del__ wrapper visible from Python.

    The rationale given in the initial commit is that there's no need to define a wrapper, since tp_del won't be populated from C code (that will use tp_dealloc instead), but there's now at least one case where it *is* populated from C (generators), which means it behaves *as if* __del__ is defined (since the interpreter actually checks the tp_del slot), but *looks* like __del__ *isn't* defined (since there is no wrapper created).

    Independent of the memory leak concerns with generators defining tp_del, it would be better if a wrapper function was defined so the existence of the method was at least visible from Python code.

    @ncoghlan ncoghlan added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Apr 20, 2013
    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Apr 20, 2013

    Would this mean that the destructor could be run more than once (or prematurely)?

    @pitrou
    Copy link
    Member

    pitrou commented Apr 20, 2013

    Sounds reasonable to me. Note that it won't remove the special-casing in gcmodule.c:

    static int
    has_finalizer(PyObject *op)
    {
        if (PyGen_CheckExact(op))
            return PyGen_NeedsFinalizing((PyGenObject *)op);
        else
            return op->ob_type->tp_del != NULL;
    }

    @benjaminp
    Copy link
    Contributor

    What exactly would calling such a wrapper do?

    @ncoghlan
    Copy link
    Contributor Author

    Calling __del__ explicitly shouldn't be any worse than doing the same thing for any other type implemented in Python (or, in the case of generators, calling close() multiple times). What I'm mostly interested in is the "can this type cause uncollectable cycles" introspection aspect.

    However, as Antoine noted, generators are an interesting special case because the GC is able to *skip* finalising them in some cases, so exposing __del__ isn't right for them either (as that suggests they will *always* be uncollectable in a cycle, when that isn't the case).

    So now I'm wondering if a better answer may be to generalise the current generator special case to a "__needsdel__" protocol: provide a __del__ method, but always make it possible for the GC to skip it when it wouldn't do anything (e.g. if you've already called close() explicitly). PyGenerator_NeedsFinalizing would then become the __needsdel__ impl for generators, and we could lose the special casing in the GC code. From Python, you could detect the three cases through:

    __del__ only: can cause uncollectable cycles
    __del__and __needsdel__: can cause uncollectable cycles, but it depends on the instance state
    Neither: can't cause uncollectable cycles

    @benjaminp
    Copy link
    Contributor

    I don't understand why we need to invent a protocol for this. The gc module already has methods and members for introspecting the collection. I don't think the gen special casing currently needs to be generalized. (What would use it?)

    @ncoghlan
    Copy link
    Contributor Author

    Yeah, I've figured out that rather than exposing __del__ if tp_del is populated, or generalising the generator special case, the simplest way to make this info accessible is to be able to ask the *garbage collector* if it thinks an object needs finalising.

    That actually makes this a pretty easy issue (as C issues go) - it's just a matter of exposing http://hg.python.org/cpython/file/default/Modules/gcmodule.c#l525 (has_finalizer) as gc.needs_finalizing.

    It will be easier once bpo-17468 is done though, since that will make it clearer what the documentation should say.

    @ncoghlan ncoghlan added the easy label Apr 20, 2013
    @ncoghlan ncoghlan changed the title Expose __del__ when tp_del is populated from C code Add gc.needs_finalizing() to check if an object needs finalising Apr 20, 2013
    @ncoghlan
    Copy link
    Contributor Author

    Antoine came up with a scheme (see bpo-17807) that should eliminate the tp_del implementation from generator objects by moving the cleanup code to the frame deallocation.

    This will restore Guido's original assumption that types implemented in C won't need to provide tp_del, so this proposal becomes obsolete.

    @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
    Labels
    easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants