gh-89373: Document that error indicator may be set in tp_dealloc by ezyang · Pull Request #28358 · python/cpython

ezyang · 2021-09-15T16:50:04Z

Signed-off-by: Edward Z. Yang ezyang@fb.com

https://bugs.python.org/issue45210

Issue: tp_dealloc docs should mention error indicator may be set #89373

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

the-knights-who-say-ni · 2021-09-15T16:50:07Z

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 username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@ezyang

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!

ezyang · 2021-09-15T16:57:42Z

just signed CLA

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

JelleZijlstra · 2022-04-02T21:20:57Z

There is a merge conflict, could you take a look?

…cator

ezyang · 2022-04-03T01:00:05Z

done

JelleZijlstra

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.

JelleZijlstra · 2022-04-03T01:20:39Z

   :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


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?

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.)

ezyang · 2022-04-03T01:37:12Z

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.

vstinner · 2022-04-05T22:24:20Z

+   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):


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().

python-cla-bot · 2025-04-06T14:31:10Z

All commit authors signed the Contributor License Agreement.

…cator

ezyang · 2025-06-06T03:13:54Z

Merge conflicts fixed LOL, three years later

Signed-off-by: Edward Z. Yang <ezyang@meta.com>

Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner · 2025-06-07T11:10:32Z

@ezyang: Oh, the last blocker point is that you didnd't sign the CLA with your email, see: #28358 (comment)

ezyang · 2025-06-09T03:17:37Z

Too many emails lol. Fixed.

vstinner

LGTM

miss-islington-app · 2025-06-09T12:53:45Z

Thanks @ezyang for the PR, and @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!

miss-islington-app · 2025-06-09T12:53:45Z

Thanks @ezyang for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington-app · 2025-06-09T12:53:47Z

Sorry, @ezyang and @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

…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>

bedevere-app · 2025-06-09T12:53:58Z

GH-135298 is a backport of this pull request to the 3.14 branch.

vstinner · 2025-06-09T12:54:45Z

Merged, thanks @ezyang. Better late than never :-)

…loc (GH-28358) (#135298) bpo-45210: Document that error indicator may be set in tp_dealloc (GH-28358) (cherry picked from commit 8441b26) Signed-off-by: Edward Z. Yang <ezyang@fb.com> Signed-off-by: Edward Z. Yang <ezyang@meta.com> Co-authored-by: Edward Z. Yang <ezyang@mit.edu> Co-authored-by: Victor Stinner <vstinner@python.org>

…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>

Document that error indicator may be set in tp_dealloc

6107190

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

the-knights-who-say-ni added the CLA not signed label Sep 15, 2021

bedevere-bot added awaiting review docs Documentation in the Doc dir labels Sep 15, 2021

add blurb

3d7a842

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Oct 3, 2021

Merge remote-tracking branch 'origin/main' into tp-dealloc-error-indi…

ef342c5

…cator

JelleZijlstra reviewed Apr 3, 2022

View reviewed changes

vstinner reviewed Apr 5, 2022

View reviewed changes

Update typeobj.rst

f068c4e

vstinner mentioned this pull request Apr 6, 2022

gh-89373: _Py_Dealloc() checks tp_dealloc exception #32357

Merged

This comment was marked as off-topic.

Sign in to view

bedevere-bot added awaiting core review and removed awaiting review labels Apr 21, 2022

ezyang mannequin mentioned this pull request Apr 27, 2022

tp_dealloc docs should mention error indicator may be set #89373

Closed

ezio-melotti removed the CLA signed label Jul 13, 2022

willingc added awaiting changes and removed awaiting core review labels Nov 4, 2024

ezyang added 2 commits June 5, 2025 22:53

Merge remote-tracking branch 'origin/main' into tp-dealloc-error-indi…

598791b

…cator

Merge remote-tracking branch 'origin' into tp-dealloc-error-indicator

bd83da0

vstinner reviewed Jun 6, 2025

View reviewed changes

Comment thread Doc/c-api/typeobj.rst Outdated

Don't use deprecated PyErr_Fetch

6c71885

Signed-off-by: Edward Z. Yang <ezyang@meta.com>

Update Doc/c-api/typeobj.rst

3e62ace

Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner reviewed Jun 9, 2025

View reviewed changes

Comment thread Doc/c-api/typeobj.rst Outdated

Update Doc/c-api/typeobj.rst

6afd215

vstinner approved these changes Jun 9, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting changes labels Jun 9, 2025

vstinner enabled auto-merge (squash) June 9, 2025 08:51

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 merged commit 8441b26 into python:main Jun 9, 2025
27 of 28 checks passed

github-project-automation Bot moved this from Todo to Done in Docs PRs Jun 9, 2025

bedevere-app Bot removed the awaiting merge label 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

miss-islington-app Bot assigned vstinner Jun 9, 2025

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 9, 2025

vstinner removed the needs backport to 3.13 bugs and security fixes label Jun 9, 2025

kcdodd mentioned this pull request Aug 22, 2025

dlpack destructor "clobbers preexisting error indicator" jax-ml/jax#31271

Closed

Uh oh!

Uh oh!

Conversation

ezyang commented Sep 15, 2021 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Sep 15, 2021

Recognized GitHub username

Uh oh!

ezyang commented Sep 15, 2021

Uh oh!

JelleZijlstra commented Apr 2, 2022

Uh oh!

ezyang commented Apr 3, 2022

Uh oh!

JelleZijlstra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Apr 3, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang Apr 3, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang commented Apr 3, 2022

Uh oh!

vstinner Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

python-cla-bot Bot commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ezyang commented Jun 6, 2025

Uh oh!

Uh oh!

vstinner commented Jun 7, 2025

Uh oh!

ezyang commented Jun 9, 2025

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app Bot commented Jun 9, 2025

Uh oh!

miss-islington-app Bot commented Jun 9, 2025

Uh oh!

miss-islington-app Bot commented Jun 9, 2025

Uh oh!

bedevere-app Bot commented Jun 9, 2025

Uh oh!

vstinner commented Jun 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ezyang commented Sep 15, 2021 •

edited by bedevere-app Bot

Loading

JelleZijlstra left a comment •

edited

Loading

python-cla-bot Bot commented Apr 6, 2025 •

edited

Loading