[Python-checkins] bpo-38206: Clarify tp_dealloc requirements for heap allocated types. (GH-16248)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/5faff977adbe089e1f91a5916ccb2160a22dd292
commit: 5faff977adbe089e1f91a5916ccb2160a22dd292
branch: master
author: Ammar Askar <ammar at ammaraskar.com>
committer: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
date: 2019-09-27T04:11:27-07:00
summary:

bpo-38206: Clarify tp_dealloc requirements for heap allocated types. (GH-16248)



As mentioned in the bpo ticket, this mistake came up on two reviews:
- https://github.com/python/cpython/pull/16127#pullrequestreview-288312751
- https://github.com/python/cpython/pull/16071#pullrequestreview-287819525

Would be nice to have it documented in a more permanent place than 3.8's whatsnew entry.


https://bugs.python.org/issue38206



Automerge-Triggered-By: @encukou

files:
M Doc/c-api/type.rst
M Doc/c-api/typeobj.rst

diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst
index 36309dd5254a..73311ec083a8 100644
--- a/Doc/c-api/type.rst
+++ b/Doc/c-api/type.rst
@@ -118,7 +118,8 @@ The following functions and structs are used to create
 
 .. c:function:: PyObject* PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
 
-   Creates and returns a heap type object from the *spec*.
+   Creates and returns a heap type object from the *spec*
+   (:const:`Py_TPFLAGS_HEAPTYPE`).
 
    If *bases* is a tuple, the created heap type contains all types contained
    in it as base types.
diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst
index 7c7a79129ccc..bb7675007430 100644
--- a/Doc/c-api/typeobj.rst
+++ b/Doc/c-api/typeobj.rst
@@ -654,9 +654,9 @@ and :c:type:`PyType_Type` effectively act as defaults.)
    the instance is still in existence, but there are no references to it.  The
    destructor function should free all references which the instance owns, free all
    memory buffers owned by the instance (using the freeing function corresponding
-   to the allocation function used to allocate the buffer), and finally (as its
-   last action) call the type's :c:member:`~PyTypeObject.tp_free` function.  If the type is not
-   subtypable (doesn't have the :const:`Py_TPFLAGS_BASETYPE` flag bit set), it is
+   to the allocation function used to allocate the buffer), and call the type's
+   :c:member:`~PyTypeObject.tp_free` function.  If the type is not subtypable
+   (doesn't have the :const:`Py_TPFLAGS_BASETYPE` flag bit set), it is
    permissible to call the object deallocator directly instead of via
    :c:member:`~PyTypeObject.tp_free`.  The object deallocator should be the one used to allocate the
    instance; this is normally :c:func:`PyObject_Del` if the instance was allocated
@@ -664,6 +664,21 @@ 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`.
 
+   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
+   recommended way to achieve this is:
+
+   .. code-block:: c
+
+     static void foo_dealloc(foo_object *self) {
+         PyTypeObject *tp = Py_TYPE(self);
+         // free references and buffers here
+         tp->tp_free(self);
+         Py_DECREF(tp);
+     }
+
+
    **Inheritance:**
 
    This field is inherited by subtypes.
@@ -1021,7 +1036,8 @@ and :c:type:`PyType_Type` effectively act as defaults.)
 
    .. data:: Py_TPFLAGS_HEAPTYPE
 
-      This bit is set when the type object itself is allocated on the heap.  In this
+      This bit is set when the type object itself is allocated on the heap, for
+      example, types created dynamically using :c:func:`PyType_FromSpec`.  In this
       case, the :attr:`ob_type` field of its instances is considered a reference to
       the type, and the type object is INCREF'ed when a new instance is created, and
       DECREF'ed when an instance is destroyed (this does not apply to instances of