bpo-31095: fix potential crash during GC (GH-2974)

1 files changed, 20 insertions, 9 deletions

diff --git a/Doc/extending/newtypes.rst b/Doc/extending/newtypes.rst
index bd09aa695d..f0e8985c61 100644
--- a/Doc/extending/newtypes.rst
+++ b/Doc/extending/newtypes.rst
@@ -728,8 +728,9 @@ functions.  With :c:func:`Py_VISIT`, :c:func:`Noddy_traverse` can be simplified:
    uniformity across these boring implementations.
 
 We also need to provide a method for clearing any subobjects that can
-participate in cycles.  We implement the method and reimplement the deallocator
-to use it::
+participate in cycles.
+
+::
 
    static int
    Noddy_clear(Noddy *self)
@@ -747,13 +748,6 @@ to use it::
        return 0;
    }
 
-   static void
-   Noddy_dealloc(Noddy* self)
-   {
-       Noddy_clear(self);
-       Py_TYPE(self)->tp_free((PyObject*)self);
-   }
-
 Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
 temporary variable so that we can set each member to *NULL* before decrementing
 its reference count.  We do this because, as was discussed earlier, if the
@@ -776,6 +770,23 @@ be simplified::
        return 0;
    }
 
+Note that :c:func:`Noddy_dealloc` may call arbitrary functions through
+``__del__`` method or weakref callback. It means circular GC can be
+triggered inside the function.  Since GC assumes reference count is not zero,
+we need to untrack the object from GC by calling :c:func:`PyObject_GC_UnTrack`
+before clearing members. Here is reimplemented deallocator which uses
+:c:func:`PyObject_GC_UnTrack` and :c:func:`Noddy_clear`.
+
+::
+
+   static void
+   Noddy_dealloc(Noddy* self)
+   {
+       PyObject_GC_UnTrack(self);
+       Noddy_clear(self);
+       Py_TYPE(self)->tp_free((PyObject*)self);
+   }
+
 Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
 
    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_flags */