gh-126907: Use a list for atexit callbacks#127935
Merged
kumaraditya303 merged 19 commits intopython:mainfrom Dec 16, 2024
Merged
gh-126907: Use a list for atexit callbacks#127935kumaraditya303 merged 19 commits intopython:mainfrom
atexit callbacks#127935kumaraditya303 merged 19 commits intopython:mainfrom
Conversation
vstinner
reviewed
Dec 16, 2024
Member
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
I like the overall change. Here is a first review.
Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
kumaraditya303
approved these changes
Dec 16, 2024
Member
|
Congrats @ZeroIntensity for this nice fix! |
kumaraditya303
added a commit
to kumaraditya303/cpython
that referenced
this pull request
Dec 17, 2024
…127935) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
kumaraditya303
added a commit
to kumaraditya303/cpython
that referenced
this pull request
Dec 17, 2024
…127935) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
srinivasreddy
pushed a commit
to srinivasreddy/cpython
that referenced
this pull request
Jan 8, 2025
…127935) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Member
|
Should this be backported or is it not worth it? |
Member
Author
|
To 3.13? I don't think it's worth risking breakage. |
This was referenced Oct 14, 2025
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 26, 2025
sagemathgh-41021: Refactor ``atexit.pyx`` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> I have refactor the ``atexit.pyx`` since for python 3.14, the changes of ``atexit`` is so much for supporting the nogil version. ### Python<=3.13 Implementation: - The ``atexit`` module stores callbacks in a C array of structs (``atexit_callback``). - The structure is defined in C as: ``` typedef struct { PyObject *func; PyObject *args; PyObject *kwargs ;} atexit_callback; ``` - Callbacks are stored in the interp->atexit.callbacks field as a C array - The array can be directly accessed from Cython code using pointer arithmetic ### Python 3.14 Implementation: - The ``atexit`` module was refactored to use a Python ``PyList`` object instead of a C array. - The structure is now: ``` state.callbacks = [(func, args, kwargs), ...] # A Python list of tuples ``` - In the C implementation, callbacks are managed with: ``` PyObject *callbacks; // This is now a PyList ``` - Callbacks are inserted at the beginning (LIFO order) using ``PyList_Insert(state->callbacks, 0, callback)`` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> python/cpython@3b76682 84d4461af python/cpython#127935 URL: sagemath#41021 Reported by: Chenxin Zhong Reviewer(s): Copilot, da-woods
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 26, 2025
sagemathgh-41021: Refactor ``atexit.pyx`` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> I have refactor the ``atexit.pyx`` since for python 3.14, the changes of ``atexit`` is so much for supporting the nogil version. ### Python<=3.13 Implementation: - The ``atexit`` module stores callbacks in a C array of structs (``atexit_callback``). - The structure is defined in C as: ``` typedef struct { PyObject *func; PyObject *args; PyObject *kwargs ;} atexit_callback; ``` - Callbacks are stored in the interp->atexit.callbacks field as a C array - The array can be directly accessed from Cython code using pointer arithmetic ### Python 3.14 Implementation: - The ``atexit`` module was refactored to use a Python ``PyList`` object instead of a C array. - The structure is now: ``` state.callbacks = [(func, args, kwargs), ...] # A Python list of tuples ``` - In the C implementation, callbacks are managed with: ``` PyObject *callbacks; // This is now a PyList ``` - Callbacks are inserted at the beginning (LIFO order) using ``PyList_Insert(state->callbacks, 0, callback)`` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> python/cpython@3b76682 84d4461af python/cpython#127935 URL: sagemath#41021 Reported by: Chenxin Zhong Reviewer(s): Copilot, da-woods
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc @colesbury, @vstinner, @kumaraditya303
This is an alternative to gh-126908, and I'm a lot happier with this. Sam's suggestion of using a list turned out to be pretty nice, with the exception of
unregisterbeing a little wonky. I suspect we could improve that a little by adding a private API for removing from a list more cleanly, but that's work for later. FWIW, both this PR and the other one will have trouble backporting due to the runtime structure changing size.atexitfrom threads in free-threading build segfaults #126907