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

[C API] Make PyGC_Head structure opaque, or even move it to the internal C API #84422

Closed
vstinner opened this issue Apr 9, 2020 · 12 comments
Closed
Labels
3.9 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

vstinner commented Apr 9, 2020

BPO 40241
Nosy @vstinner, @pablogsal, @miss-islington
PRs
  • bpo-40241: Move SIZEOF_PYGC_HEAD to _testinternalcapi #19452
  • bpo-40241: Add PyObject_GC_IsTracked and PyObject_GC_IsFinalized to the public C-API #19461
  • bpo-40241: Add pycore_gc.h header file #19494
  • bpo-40241: Add pycore_interp.h header #19499
  • bpo-40268: Include explicitly pycore_interp.h #19505
  • bpo-40241: What's New in Python 3.9: opaque PyGC_Head #20586
  • [3.9] bpo-40241: What's New in Python 3.9: opaque PyGC_Head (GH-20586) #20593
  • 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 2020-04-13.10:20:38.319>
    created_at = <Date 2020-04-09.15:19:56.824>
    labels = ['expert-C-API', '3.9']
    title = '[C API] Make PyGC_Head structure opaque, or even move it to the internal C API'
    updated_at = <Date 2020-06-02.10:09:35.483>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-06-02.10:09:35.483>
    actor = 'miss-islington'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-04-13.10:20:38.319>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2020-04-09.15:19:56.824>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40241
    keywords = ['patch']
    message_count = 12.0
    messages = ['366064', '366067', '366092', '366094', '366095', '366170', '366291', '366296', '366300', '366353', '370600', '370601']
    nosy_count = 3.0
    nosy_names = ['vstinner', 'pablogsal', 'miss-islington']
    pr_nums = ['19452', '19461', '19494', '19499', '19505', '20586', '20593']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue40241'
    versions = ['Python 3.9']

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 9, 2020

    Similarly to bpo-39573 (PyObject) and bpo-40170 (PyTypeObject), I propose to make the PyGC_Head structure opaque in the C API.

    See https://bugs.python.org/issue39573#msg361513 for the rationale. In short, my plan is to hide all implementation details from the C API.

    The PyGC_Head structure caused ABI issues recently: bpo-39599 "ABI breakage between Python 3.7.4 and 3.7.5: change in PyGC_Head structure". Making the structure opaque would reduce the risk of such ABI issue. In fact, the reporter of bpo-39599 really require to access PyGC_Head structure to write a profiler, so this issue doesn't fix all use cases, but it should benefit to most people ;-) PyGC_Head structure will remain accessible via the internal C API which doesn't provide any backward compatibility warranty.

    @vstinner vstinner added 3.9 only security fixes topic-C-API labels Apr 9, 2020
    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 9, 2020

    See also bpo-40240: "Expose public spelling of _PyGC_FINALIZED and _PyGC_SET_FINALIZED?".

    @vstinner
    Copy link
    Member Author

    The following macros rely on PyGC_Head:

    • _PyGCHead_FINALIZED()
    • _PyGCHead_NEXT()
    • _PyGCHead_PREV()
    • _PyGCHead_SET_FINALIZED()
    • _PyGCHead_SET_NEXT()
    • _PyGCHead_SET_PREV()
    • _PyGC_FINALIZED()
    • _PyGC_PREV_MASK
    • _PyGC_PREV_MASK_COLLECTING
    • _PyGC_PREV_MASK_FINALIZED
    • _PyGC_PREV_SHIFT
    • _PyGC_SET_FINALIZED()
    • _PyObject_GC_IS_TRACKED()
    • _PyObject_GC_MAY_BE_TRACKED()
    • _Py_AS_GC()

    _testcapi uses _PyObject_GC_IS_TRACKED() and sizeof(PyGC_Head).

    @vstinner
    Copy link
    Member Author

    The following macros rely on PyGC_Head: (...)

    I already moved them to the internal C API in commit 1a6be91 of bpo-35081.

    But Stefan Behnel reported that it breaks Cython:

    "Making _PyGC_FINALIZED() internal broke Cython (cython/cython#2721). It's used in the finaliser implementation (https://github.com/cython/cython/blob/da657c8e326a419cde8ae6ea91be9661b9622504/Cython/Compiler/ModuleNode.py#L1442-L1456), to determine if an object for which tp_dealloc() is called has already been finalised or whether we have to do it. I'm not sure how to deal with this on our side now. Any clue?"

    https://bugs.python.org/issue35081#msg330045

    So I simply reverted (partially) this change: commit 3e21ad1.

    @vstinner
    Copy link
    Member Author

    _testcapi uses _PyObject_GC_IS_TRACKED(). This macro is exposed in Python as gc.is_tracked(). IMO the function should be available in the public C API. For example, PyObject_GC_IsTracked(obj).

    Cython uses _PyGC_FINALIZED(). This macro is exposed in Python as gc.is_finalized(). So again, I consider that it should be exposed in a public C function as well, like PyObject_GC_IsFinalized(obj).

    @pppery pppery mannequin changed the title [C API] Make PyGC_Head structure opaque, or even more it to the internal C API [C API] Make PyGC_Head structure opaque, or even move it to the internal C API Apr 10, 2020
    @pppery pppery mannequin changed the title [C API] Make PyGC_Head structure opaque, or even more it to the internal C API [C API] Make PyGC_Head structure opaque, or even move it to the internal C API Apr 10, 2020
    @pablogsal
    Copy link
    Member

    New changeset f13072b by Pablo Galindo in branch 'master':
    bpo-40241: Add PyObject_GC_IsTracked and PyObject_GC_IsFinalized to the public C-API (GH-19461)
    f13072b

    @vstinner
    Copy link
    Member Author

    New changeset 0135598 by Victor Stinner in branch 'master':
    bpo-40241: Add pycore_gc.h header file (GH-19494)
    0135598

    @vstinner
    Copy link
    Member Author

    Ok, this issue should now be closed. It was easier than expected ;-)

    @vstinner
    Copy link
    Member Author

    New changeset 0c13e1f by Victor Stinner in branch 'master':
    bpo-40241: Add pycore_interp.h header (GH-19499)
    0c13e1f

    @vstinner
    Copy link
    Member Author

    bpo-40241: Add pycore_interp.h header (GH-19499)

    Oops, this change was for bpo-40268.

    @vstinner
    Copy link
    Member Author

    vstinner commented Jun 2, 2020

    New changeset 337d310 by Victor Stinner in branch 'master':
    bpo-40241: What's New in Python 3.9: opaque PyGC_Head (GH-20586)
    337d310

    @miss-islington
    Copy link
    Contributor

    New changeset ff442f3 by Miss Islington (bot) in branch '3.9':
    bpo-40241: What's New in Python 3.9: opaque PyGC_Head (GH-20586)
    ff442f3

    @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
    3.9 only security fixes topic-C-API
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants