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

[patch] assert tp_traverse in PyType_GenericAlloc() #46003

Closed
bsilverthorn mannequin opened this issue Dec 19, 2007 · 9 comments
Closed

[patch] assert tp_traverse in PyType_GenericAlloc() #46003

bsilverthorn mannequin opened this issue Dec 19, 2007 · 9 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@bsilverthorn
Copy link
Mannequin

bsilverthorn mannequin commented Dec 19, 2007

BPO 1662
Nosy @pitrou, @vstinner, @devdanzin, @iritkatriel
Files
  • bcs_assert_tp_traverse_r72055.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 2021-05-14.11:10:56.549>
    created_at = <Date 2007-12-19.17:22:01.646>
    labels = ['interpreter-core', 'type-bug']
    title = '[patch] assert tp_traverse in PyType_GenericAlloc()'
    updated_at = <Date 2021-06-29.01:32:27.631>
    user = 'https://bugs.python.org/bsilverthorn'

    bugs.python.org fields:

    activity = <Date 2021-06-29.01:32:27.631>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-05-14.11:10:56.549>
    closer = 'iritkatriel'
    components = ['Interpreter Core']
    creation = <Date 2007-12-19.17:22:01.646>
    creator = 'bsilverthorn'
    dependencies = []
    files = ['13800']
    hgrepos = []
    issue_num = 1662
    keywords = ['patch']
    message_count = 9.0
    messages = ['58811', '86708', '86712', '86899', '392243', '392277', '393639', '396689', '396690']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'ajaksu2', 'bsilverthorn', 'iritkatriel']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1662'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @bsilverthorn
    Copy link
    Mannequin Author

    bsilverthorn mannequin commented Dec 19, 2007

    Attached is a very short patch against r59568 which asserts tp_traverse
    on (the types of) objects allocated in PyType_GenericAlloc(). As far as
    I'm aware, tp_traverse should always be set at this point. Catching that
    error early, even if only in debug builds, would help to prevent bugs
    like http://bugzilla.gnome.org/show_bug.cgi?id=504337 .

    @bsilverthorn bsilverthorn mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Dec 19, 2007
    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Apr 27, 2009

    Bryan: can you provide a test case without external dependencies? If
    not, confirming this is still valid in 2.6 would also help.

    @bsilverthorn
    Copy link
    Mannequin Author

    bsilverthorn mannequin commented Apr 28, 2009

    Well, there's no Python bug per se, hence no test case; this patch just
    adds a single additional assert that might catch a particular extension
    implementation mistake. It was prompted by tracking down the bug in
    pygtk mentioned above.

    I've attached an updated patch against r72055. It's a trivial change,
    but I would suggest that someone more familiar with the Python core sign
    off on it regardless.

    @pitrou
    Copy link
    Member

    pitrou commented May 1, 2009

    It would be probably be better to put this check in PyType_Ready() instead.

    @iritkatriel
    Copy link
    Member

    Are you sure this assertion is correct? tp_traverse is optional.

    @bsilverthorn
    Copy link
    Mannequin Author

    bsilverthorn mannequin commented Apr 29, 2021

    I submitted this patch 14 years ago and am sure of nothing. :)

    @iritkatriel
    Copy link
    Member

    tp_traverse is optional, so we should not add this assertion.

    @vstinner
    Copy link
    Member

    This issue was fixed differently in bpo-44263, by adding a test in PyType_Ready():

        // bpo-44263: tp_traverse is required if Py_TPFLAGS_HAVE_GC is set.
        // Note: tp_clear is optional.
        if (type->tp_flags & Py_TPFLAGS_HAVE_GC
            && type->tp_traverse == NULL)
        {
            PyErr_Format(PyExc_SystemError,
                         "type %s has the Py_TPFLAGS_HAVE_GC flag "
                         "but has no traverse function",
                         type->tp_name);
            return -1;
        }

    commit ee76375
    Author: Victor Stinner <vstinner@python.org>
    Date: Tue Jun 1 23:37:12 2021 +0200

    bpo-44263: Py_TPFLAGS_HAVE_GC requires tp_traverse (GH-26463)
    
    The PyType_Ready() function now raises an error if a type is defined
    with the Py_TPFLAGS_HAVE_GC flag set but has no traverse function
    (PyTypeObject.tp_traverse).
    

    @vstinner
    Copy link
    Member

    tp_traverse is optional, so we should not add this assertion.

    It is required by types implementing the GC protocol. This assumption is non-obvious and was not documented. The documentation was completed in bpo-44263 by commit 8b55bc3.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants