-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-89373: Document that error indicator may be set in tp_dealloc #28358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
6107190
3d7a842
ef342c5
f068c4e
598791b
bd83da0
6c71885
3e62ace
6afd215
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -686,6 +686,24 @@ and :c:data:`PyType_Type` effectively act as defaults.) | |
instance, and call the type's :c:member:`~PyTypeObject.tp_free` function to | ||
free the object itself. | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! |
||
|
||
.. code-block:: c | ||
|
||
static void foo_dealloc(foo_object *self) { | ||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
PyObject *et, *ev, *etb; | ||
PyErr_Fetch(&et, &ev, &etb); | ||
ezyang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
... | ||
PyErr_Restore(et, ev, etb); | ||
} | ||
|
||
The dealloc handler itself must not raise an exception; if it hits an error | ||
case it should call :c:func:`PyErr_WriteUnraisable` to log (and clear) an | ||
ezyang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
unraisable exception. | ||
|
||
No guarantees are made about when an object is destroyed, except: | ||
|
||
* Python will destroy an object immediately or some time after the final | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Document that error indicator may be set in tp_dealloc, and how to avoid | ||
clobbering it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)