gh-89373: Document that error indicator may be set in tp_dealloc #28358
Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!
Check out: eBank.nz (Art Generator) |
Netwrck.com (AI Tools) |
Text-Generator.io (AI API) |
BitBank.nz (Crypto AI) |
ReadingTime (Kids Reading) |
RewordGame |
BigMultiplayerChess |
WebFiddle |
How.nz |
Helix AI Assistant
Conversation
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
just signed CLA |
|
There is a merge conflict, could you take a look? |
|
done |
There was a problem hiding this comment.
Thanks. This note seems useful, but I am not that comfortable with the C API, and on the bug @vstinner seems to prefer a different direction, so this will need more discussion.
Doc/c-api/typeobj.rst
Outdated
| @@ -668,6 +668,20 @@ and :c:type:`PyType_Type` effectively act as defaults.) | |||
| :c:func:`PyObject_GC_Del` if the instance was allocated using | |||
| :c:func:`PyObject_GC_New` or :c:func:`PyObject_GC_NewVar`. | |||
|
|
|||
| If you may call functions that may set the error indicator, you must | |||
There was a problem hiding this comment.
As I understand it, very little of the C API is safe to call with an active error set. So maybe this should be a stronger message?
There was a problem hiding this comment.
IIRC, the intent here was not to discuss C API functions, but if you were calling into userland (which happens if you've got some complicated cleanup functions; at least that's what bit us here.)
|
I don't know enough about CPython to know if @vstinner's suggestion to enforce deallocation is never called when error is set is feasible. Seems... difficult to enforce. |
Doc/c-api/typeobj.rst
Outdated
| If you may call functions that may set the error indicator, you must | ||
| use :c:func:`PyErr_Fetch` and :c:func:`PyErr_Restore` to ensure you | ||
| don't clobber a preexisting error indicator (the deallocation could | ||
| have occurred while processing a different error): |
There was a problem hiding this comment.
I would add that the function must not raise an exception. It can use PyErr_WriteUnraisable() to log (and clear) an "unraisable" exception.
By the way, I'm surprised that _Py_Dealloc() doesn't ensure in debug mode (Py_DEBUG) that tp_dealloc does not raise a new exception. See also _Py_CheckSlotResult() and _Py_CheckFunctionResult().
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
All commit authors signed the Contributor License Agreement. |
|
Merge conflicts fixed LOL, three years later |
Signed-off-by: Edward Z. Yang <ezyang@meta.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
|
Too many emails lol. Fixed. |
picnixz
changed the title
bpo-45210: Document that error indicator may be set in tp_dealloc
gh-89373: Document that error indicator may be set in tp_dealloc
Jun 9, 2025
vstinner
added
needs backport to 3.13
bugs and security fixes
needs backport to 3.14
bugs and security fixes
labels
Jun 9, 2025
Thanks @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
Thanks @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖
Sorry, @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8441b263af964f353bf02d56c32a4fc547cdc330 3.13
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Jun 9, 2025
…thonGH-28358)
(cherry picked from commit 8441b26)
Co-authored-by: Edward Z. Yang <ezyang@mit.edu>
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Signed-off-by: Edward Z. Yang <ezyang@meta.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Merged, thanks @ezyang. Better late than never :-)
lkollar
pushed a commit
to lkollar/cpython
that referenced
this pull request
Jun 19, 2025
…thon#28358)
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Signed-off-by: Edward Z. Yang <ezyang@meta.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Edward Z. Yang ezyang@fb.com
https://bugs.python.org/issue45210