This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: tp_dealloc docs should mention error indicator may be set
Type: enhancement Stage: patch review
Components: Documentation Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, ezyang, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2021-09-15 16:47 by ezyang, last changed 2022-04-11 14:59 by admin.

Files
File name Uploaded Description Edit
unicode_dealloc_bug.patch vstinner, 2022-04-06 09:40
Pull Requests
URL Status Linked Edit
PR 28358 open python-dev, 2021-09-15 16:50
PR 32357 open vstinner, 2022-04-06 09:38
Messages (3)
msg401854 - (view) Author: Edward Yang (ezyang) * Date: 2021-09-15 16:47
The fact that the error indicator may be set during tp_dealloc is somewhat well known (https://github.com/posborne/dbus-python/blob/fef4bccfc535c6c2819e3f15384600d7bc198bc5/_dbus_bindings/conn.c#L387) but it's not documented in the official manual. We should document it.

A simple suggested patch:


diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst
index b17fb22b69..e7c9b13646 100644
--- a/Doc/c-api/typeobj.rst
+++ b/Doc/c-api/typeobj.rst
@@ -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
+   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):
+
+   .. code-block:: c
+
+     static void foo_dealloc(foo_object *self) {
+         PyObject *et, *ev, *etb;
+         PyErr_Fetch(&et, &ev, &etb);
+         ...
+         PyErr_Restore(et, ev, etb);
+     }
+
    Finally, if the type is heap allocated (:const:`Py_TPFLAGS_HEAPTYPE`), the
    deallocator should decrement the reference count for its type object after
    calling the type deallocator. In order to avoid dangling pointers, the
msg401944 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-16 13:21
I'm not sure that it's a feature. Maybe we should modify to never call tp_dealloc with an exception set.
msg416849 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-04-06 09:40
I wrote GH-32357 to check in debug mode that tp_dealloc functions leave the current exception unchanged. You can use attached unicode_dealloc_bug.patch to inject a bug to test my PR. Example ("make" is enough to trigger the bug):
---
Fatal Python error: _Py_Dealloc: Deallocator of type 'str' cleared the current exception
Python runtime state: initialized

Current thread 0x00007ff28d45a740 (most recent call first):
  File "/home/vstinner/python/main/Lib/sysconfig.py", line 349 in _parse_makefile
  File "/home/vstinner/python/main/Lib/sysconfig.py", line 470 in _generate_posix_vars
  File "/home/vstinner/python/main/Lib/sysconfig.py", line 845 in _main
  File "/home/vstinner/python/main/Lib/sysconfig.py", line 857 in <module>
  File "<frozen runpy>", line 88 in _run_code
  File "<frozen runpy>", line 198 in _run_module_as_main
generate-posix-vars failed
---
History
Date User Action Args
2022-04-11 14:59:50adminsetgithub: 89373
2022-04-06 09:40:23vstinnersetfiles: + unicode_dealloc_bug.patch

messages: + msg416849
2022-04-06 09:38:25vstinnersetpull_requests: + pull_request30407
2021-09-16 13:21:42vstinnersetnosy: + vstinner
messages: + msg401944
2021-09-15 16:50:05python-devsetkeywords: + patch
nosy: + python-dev

pull_requests: + pull_request26772
stage: patch review
2021-09-15 16:47:23ezyangcreate