[issue45210] tp_dealloc docs should mention error indicator may be set
New submission from Edward Yang <ezyang@mit.edu>: The fact that the error indicator may be set during tp_dealloc is somewhat well known (https://github.com/posborne/dbus-python/blob/fef4bccfc535c6c2819e3f15384600d...) 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 ---------- assignee: docs@python components: Documentation messages: 401854 nosy: docs@python, ezyang priority: normal severity: normal status: open title: tp_dealloc docs should mention error indicator may be set type: enhancement versions: Python 3.11 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue45210> _______________________________________
Change by Roundup Robot <devnull@psf.upfronthosting.co.za>: ---------- keywords: +patch nosy: +python-dev nosy_count: 2.0 -> 3.0 pull_requests: +26772 stage: -> patch review pull_request: https://github.com/python/cpython/pull/28358 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue45210> _______________________________________
STINNER Victor <vstinner@python.org> added the comment: I'm not sure that it's a feature. Maybe we should modify to never call tp_dealloc with an exception set. ---------- nosy: +vstinner _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue45210> _______________________________________
Change by STINNER Victor <vstinner@python.org>: ---------- pull_requests: +30407 pull_request: https://github.com/python/cpython/pull/32357 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue45210> _______________________________________
STINNER Victor <vstinner@python.org> added the comment: 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 --- ---------- Added file: https://bugs.python.org/file50724/unicode_dealloc_bug.patch _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue45210> _______________________________________
participants (3)
-
Edward Yang
-
Roundup Robot
-
STINNER Victor