Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 21, 2022
+50 −3
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Doc/using/configure.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ Effects of a debug build:
to detect usage of uninitialized objects.
* Ensure that functions which can clear or replace the current exception are
not called with an exception raised.
* Check that deallocator functions don't change the current exception.
* The garbage collector (:func:`gc.collect` function) runs some basic checks
on objects consistency.
* The :c:macro:`Py_SAFE_DOWNCAST()` macro checks for integer underflow and
Expand Down
14 changes: 12 additions & 2 deletions Lib/test/test_capi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,12 @@ def __del__(self):
del subclass_instance

# Test that setting __class__ modified the reference counts of the types
self.assertEqual(type_refcnt - 1, B.refcnt_in_del)
if Py_DEBUG:
# gh-89373: In debug mode, _Py_Dealloc() keeps a strong reference
# to the type while calling tp_dealloc()
self.assertEqual(type_refcnt, B.refcnt_in_del)
else:
self.assertEqual(type_refcnt - 1, B.refcnt_in_del)
self.assertEqual(new_type_refcnt + 1, A.refcnt_in_del)

# Test that the original type already has decreased its refcnt
Expand Down Expand Up @@ -581,7 +586,12 @@ def test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_o
del subclass_instance

# Test that setting __class__ modified the reference counts of the types
self.assertEqual(type_refcnt - 1, _testcapi.HeapCTypeSubclassWithFinalizer.refcnt_in_del)
if Py_DEBUG:
# gh-89373: In debug mode, _Py_Dealloc() keeps a strong reference
# to the type while calling tp_dealloc()
self.assertEqual(type_refcnt, _testcapi.HeapCTypeSubclassWithFinalizer.refcnt_in_del)
else:
self.assertEqual(type_refcnt - 1, _testcapi.HeapCTypeSubclassWithFinalizer.refcnt_in_del)
self.assertEqual(new_type_refcnt + 1, _testcapi.HeapCTypeSubclass.refcnt_in_del)

# Test that the original type already has decreased its refcnt
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-04-21-16-15-24.gh-issue-89373.A1jgLx.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
If Python is built in debug mode, Python now ensures that deallocator
functions leave the current exception unchanged. Patch by Victor Stinner.
36 changes: 35 additions & 1 deletion Objects/object.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2354,11 +2354,45 @@ _PyObject_AssertFailed(PyObject *obj, const char *expr, const char *msg,
void
_Py_Dealloc(PyObject *op)
{
destructor dealloc = Py_TYPE(op)->tp_dealloc;
PyTypeObject *type = Py_TYPE(op);
destructor dealloc = type->tp_dealloc;
#ifdef Py_DEBUG
PyThreadState *tstate = _PyThreadState_GET();
PyObject *old_exc_type = tstate->curexc_type;
// Keep the old exception type alive to prevent undefined behavior
// on (tstate->curexc_type != old_exc_type) below
Py_XINCREF(old_exc_type);
// Make sure that type->tp_name remains valid
Py_INCREF(type);
#endif

#ifdef Py_TRACE_REFS
_Py_ForgetReference(op);
#endif
(*dealloc)(op);

#ifdef Py_DEBUG
// gh-89373: The tp_dealloc function must leave the current exception
// unchanged.
if (tstate->curexc_type != old_exc_type) {
const char *err;
if (old_exc_type == NULL) {
err = "Deallocator of type '%s' raised an exception";
}
else if (tstate->curexc_type == NULL) {
err = "Deallocator of type '%s' cleared the current exception";
}
else {
// It can happen if dealloc() normalized the current exception.
// A deallocator function must not change the current exception,
// not even normalize it.
err = "Deallocator of type '%s' overrode the current exception";
}
_Py_FatalErrorFormat(__func__, err, type->tp_name);
}
Py_XDECREF(old_exc_type);
Py_DECREF(type);
#endif
}


Expand Down