[Python-checkins] bpo-38631: Replace Py_FatalError() with _PyObject_ASSERT_FAILED_MSG() (GH-18258)

Victor Stinner webhook-mailer at python.org
Thu Jan 30 03:01:23 EST 2020


https://github.com/python/cpython/commit/5eb8bff7e4aa7e4d8580a30323641388c8ff59a5
commit: 5eb8bff7e4aa7e4d8580a30323641388c8ff59a5
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020-01-30T09:01:07+01:00
summary:

bpo-38631: Replace Py_FatalError() with _PyObject_ASSERT_FAILED_MSG() (GH-18258)

Replace Py_FatalError() with _PyObject_ASSERT_FAILED_MSG() in
object.c and typeobject.c to also dump the involved Python object on
a fatal error. It should ease debug when such fatal error occurs.

If the double linked list is inconsistent, _Py_ForgetReference() no
longer dumps previous and next objects in the fatal error, it now
only dumps the current object. It ensures that the error message
is displayed even if dumping the object does crash Python.

Enhance _Py_ForgetReference() error messages;
_PyObject_ASSERT_FAILED_MSG() logs the "_Py_ForgetReference" function
name.

files:
M Objects/object.c
M Objects/typeobject.c

diff --git a/Objects/object.c b/Objects/object.c
index 14533dba16d64..aa84114c55471 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -315,30 +315,30 @@ PyObject_CallFinalizer(PyObject *self)
 int
 PyObject_CallFinalizerFromDealloc(PyObject *self)
 {
-    Py_ssize_t refcnt;
-
-    /* Temporarily resurrect the object. */
     if (self->ob_refcnt != 0) {
-        Py_FatalError("PyObject_CallFinalizerFromDealloc called on "
-                      "object with a non-zero refcount");
+        _PyObject_ASSERT_FAILED_MSG(self,
+                                    "PyObject_CallFinalizerFromDealloc called "
+                                    "on object with a non-zero refcount");
     }
+
+    /* Temporarily resurrect the object. */
     self->ob_refcnt = 1;
 
     PyObject_CallFinalizer(self);
 
-    /* Undo the temporary resurrection; can't use DECREF here, it would
-     * cause a recursive call.
-     */
     _PyObject_ASSERT_WITH_MSG(self,
                               self->ob_refcnt > 0,
                               "refcount is too small");
-    if (--self->ob_refcnt == 0)
+
+    /* Undo the temporary resurrection; can't use DECREF here, it would
+     * cause a recursive call. */
+    if (--self->ob_refcnt == 0) {
         return 0;         /* this is the normal path out */
+    }
 
     /* tp_finalize resurrected it!  Make it look like the original Py_DECREF
-     * never happened.
-     */
-    refcnt = self->ob_refcnt;
+     * never happened. */
+    Py_ssize_t refcnt = self->ob_refcnt;
     _Py_NewReference(self);
     self->ob_refcnt = refcnt;
 
@@ -352,8 +352,7 @@ PyObject_CallFinalizerFromDealloc(PyObject *self)
      * chain, so no more to do there.
      * If COUNT_ALLOCS, the original decref bumped tp_frees, and
      * _Py_NewReference bumped tp_allocs:  both of those need to be
-     * undone.
-     */
+     * undone. */
 #ifdef COUNT_ALLOCS
     --Py_TYPE(self)->tp_frees;
     --Py_TYPE(self)->tp_allocs;
@@ -1938,29 +1937,30 @@ _Py_NewReference(PyObject *op)
 void
 _Py_ForgetReference(PyObject *op)
 {
-#ifdef SLOW_UNREF_CHECK
-    PyObject *p;
-#endif
-    if (op->ob_refcnt < 0)
-        Py_FatalError("UNREF negative refcnt");
+    if (op->ob_refcnt < 0) {
+        _PyObject_ASSERT_FAILED_MSG(op, "negative refcnt");
+    }
+
     if (op == &refchain ||
-        op->_ob_prev->_ob_next != op || op->_ob_next->_ob_prev != op) {
-        fprintf(stderr, "* ob\n");
-        _PyObject_Dump(op);
-        fprintf(stderr, "* op->_ob_prev->_ob_next\n");
-        _PyObject_Dump(op->_ob_prev->_ob_next);
-        fprintf(stderr, "* op->_ob_next->_ob_prev\n");
-        _PyObject_Dump(op->_ob_next->_ob_prev);
-        Py_FatalError("UNREF invalid object");
+        op->_ob_prev->_ob_next != op || op->_ob_next->_ob_prev != op)
+    {
+        _PyObject_ASSERT_FAILED_MSG(op, "invalid object chain");
     }
+
 #ifdef SLOW_UNREF_CHECK
+    PyObject *p;
     for (p = refchain._ob_next; p != &refchain; p = p->_ob_next) {
-        if (p == op)
+        if (p == op) {
             break;
+        }
+    }
+    if (p == &refchain) {
+        /* Not found */
+        _PyObject_ASSERT_FAILED_MSG(op,
+                                    "object not found in the objects list");
     }
-    if (p == &refchain) /* Not found */
-        Py_FatalError("UNREF unknown object");
 #endif
+
     op->_ob_next->_ob_prev = op->_ob_prev;
     op->_ob_prev->_ob_next = op->_ob_next;
     op->_ob_next = op->_ob_prev = NULL;
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 720363410ceb1..5773eb7096e0c 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3570,9 +3570,9 @@ type_traverse(PyTypeObject *type, visitproc visit, void *arg)
        for heaptypes. */
     if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
         char msg[200];
-        sprintf(msg, "type_traverse() called for non-heap type '%.100s'",
+        sprintf(msg, "type_traverse() called on non-heap type '%.100s'",
                 type->tp_name);
-        Py_FatalError(msg);
+        _PyObject_ASSERT_FAILED_MSG((PyObject *)type, msg);
     }
 
     Py_VISIT(type->tp_dict);



More information about the Python-checkins mailing list