GH-124715: trashcan protection for all GC objects

Include/cpython/object.h

@@ -418,83 +418,33 @@ PyAPI_FUNC(void) _Py_NO_RETURN _PyObject_AssertFailed(
     const char *function);
 
 
-/* Trashcan mechanism, thanks to Christian Tismer.
-
-When deallocating a container object, it's possible to trigger an unbounded
-chain of deallocations, as each Py_DECREF in turn drops the refcount on "the
-next" object in the chain to 0.  This can easily lead to stack overflows,
-especially in threads (which typically have less stack space to work with).
-
-A container object can avoid this by bracketing the body of its tp_dealloc
-function with a pair of macros:
-
-static void
-mytype_dealloc(mytype *p)
-{
-    ... declarations go here ...
-
-    PyObject_GC_UnTrack(p);        // must untrack first
-    Py_TRASHCAN_BEGIN(p, mytype_dealloc)
-    ... The body of the deallocator goes here, including all calls ...
-    ... to Py_DECREF on contained objects.                         ...
-    Py_TRASHCAN_END                // there should be no code after this
-}
-
-CAUTION:  Never return from the middle of the body!  If the body needs to
-"get out early", put a label immediately before the Py_TRASHCAN_END
-call, and goto it.  Else the call-depth counter (see below) will stay
-above 0 forever, and the trashcan will never get emptied.
-
-How it works:  The BEGIN macro increments a call-depth counter.  So long
-as this counter is small, the body of the deallocator is run directly without
-further ado.  But if the counter gets large, it instead adds p to a list of
-objects to be deallocated later, skips the body of the deallocator, and
-resumes execution after the END macro.  The tp_dealloc routine then returns
-without deallocating anything (and so unbounded call-stack depth is avoided).
-
-When the call stack finishes unwinding again, code generated by the END macro
-notices this, and calls another routine to deallocate all the objects that
-may have been added to the list of deferred deallocations.  In effect, a
-chain of N deallocations is broken into (N-1)/(Py_TRASHCAN_HEADROOM-1) pieces,
-with the call stack never exceeding a depth of Py_TRASHCAN_HEADROOM.
-
-Since the tp_dealloc of a subclass typically calls the tp_dealloc of the base
-class, we need to ensure that the trashcan is only triggered on the tp_dealloc
-of the actual class being deallocated. Otherwise we might end up with a
-partially-deallocated object. To check this, the tp_dealloc function must be
-passed as second argument to Py_TRASHCAN_BEGIN().
-*/
-
-/* Python 3.9 private API, invoked by the macros below. */
+/* Python 3.9 private API */
 PyAPI_FUNC(int) _PyTrash_begin(PyThreadState *tstate, PyObject *op);
 PyAPI_FUNC(void) _PyTrash_end(PyThreadState *tstate);
 
 PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op);
 PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(PyThreadState *tstate);
 
 
-/* Python 3.10 private API, invoked by the Py_TRASHCAN_BEGIN(). */
+/* Python 3.10 private API */
 
 /* To avoid raising recursion errors during dealloc trigger trashcan before we reach
  * recursion limit. To avoid trashing, we don't attempt to empty the trashcan until
- * we have headroom above the trigger limit */
+ * we have headroom above the trigger limit
+ *
+ * FIXME: could this be moved into object.c?
+ */
 #define Py_TRASHCAN_HEADROOM 50
 
+/* Backwards compatibility macros.  The trashcan mechanism is now integrated
+ * with _Py_Dealloc and so these do nothing.  The assert() is present to handle
+ * the case that a 'goto' statement precedes Py_TRASHCAN_END.  */
 #define Py_TRASHCAN_BEGIN(op, dealloc) \
-do { \
-    PyThreadState *tstate = PyThreadState_Get(); \
-    if (tstate->c_recursion_remaining <= Py_TRASHCAN_HEADROOM && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
-        _PyTrash_thread_deposit_object(tstate, (PyObject *)op); \
-        break; \
-    } \
-    tstate->c_recursion_remaining--;
-    /* The body of the deallocator is here. */
+    do {
+
 #define Py_TRASHCAN_END \
-    tstate->c_recursion_remaining++; \
-    if (tstate->delete_later && tstate->c_recursion_remaining > (Py_TRASHCAN_HEADROOM*2)) { \
-        _PyTrash_thread_destroy_chain(tstate); \
-    } \
-} while (0);
+    assert(1); \
+    } while (0);
 
 
 PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj);

Misc/NEWS.d/next/Core_and_Builtins/2024-09-27-15-41-16.gh-issue-124715.uzszn9.rst

@@ -0,0 +1,7 @@
+Protect against stack overflows in tp_dealloc for all types that implement
+GC (i.e. the "trashcan" mechanism).  The `Py_TRASHCAN_BEGIN` and
+`Py_TRASHCAN_END` macros are now only present for backwards compatibility and
+don't need to be used. A suble API change is that tp_dealloc methods are now
+called with GC objects untracked.  Previously they would be tracked if the type
+had them originally tracked.  `PyObject_CallFinalizerFromDealloc()` will now
+track GC objects if they are resurrected.

Modules/_io/bufferedio.c

@@ -412,9 +412,12 @@ buffered_dealloc(buffered *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
     self->finalizing = 1;
-    if (_PyIOBase_finalize((PyObject *) self) < 0)
+    if (_PyIOBase_finalize((PyObject *) self) < 0) {
+        // if the object is resurrected, it will be tracked again by
+        // PyObject_CallFinalizerFromDealloc()
+        assert(_PyObject_GC_IS_TRACKED(self));
         return;
-    _PyObject_GC_UNTRACK(self);
+    }
     self->ok = 0;
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)self);
@@ -2298,7 +2301,6 @@ static void
 bufferedrwpair_dealloc(rwpair *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
-    _PyObject_GC_UNTRACK(self);
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)self);
     (void)bufferedrwpair_clear(self);

Modules/_io/bytesio.c

@@ -886,7 +886,6 @@ static void
 bytesio_dealloc(bytesio *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
-    _PyObject_GC_UNTRACK(self);
     if (self->exports > 0) {
         PyErr_SetString(PyExc_SystemError,
                         "deallocated BytesIO object has exported buffers");

Modules/_io/fileio.c

@@ -558,9 +558,12 @@ fileio_dealloc(fileio *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
     self->finalizing = 1;
-    if (_PyIOBase_finalize((PyObject *) self) < 0)
+    if (_PyIOBase_finalize((PyObject *) self) < 0) {
+        // if the object is resurrected, it will be tracked again by
+        // PyObject_CallFinalizerFromDealloc()
+        assert(_PyObject_GC_IS_TRACKED(self));
         return;
-    _PyObject_GC_UNTRACK(self);
+    }
     if (self->stat_atopen != NULL) {
         PyMem_Free(self->stat_atopen);
         self->stat_atopen = NULL;

Modules/_io/iobase.c

@@ -372,10 +372,12 @@ iobase_dealloc(iobase *self)
         if (_PyType_HasFeature(Py_TYPE(self), Py_TPFLAGS_HEAPTYPE)) {
             Py_INCREF(Py_TYPE(self));
         }
+        // if the object is resurrected, it will be tracked again by
+        // PyObject_CallFinalizerFromDealloc()
+        assert(_PyObject_GC_IS_TRACKED(self));
         return;
     }
     PyTypeObject *tp = Py_TYPE(self);
-    _PyObject_GC_UNTRACK(self);
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *) self);
     Py_CLEAR(self->dict);

Modules/_io/stringio.c

@@ -609,7 +609,6 @@ static void
 stringio_dealloc(stringio *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
-    _PyObject_GC_UNTRACK(self);
     self->ok = 0;
     if (self->buf) {
         PyMem_Free(self->buf);

Modules/_io/textio.c

@@ -284,7 +284,6 @@ static void
 incrementalnewlinedecoder_dealloc(nldecoder_object *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
-    _PyObject_GC_UNTRACK(self);
     (void)incrementalnewlinedecoder_clear(self);
     tp->tp_free((PyObject *)self);
     Py_DECREF(tp);
@@ -1456,10 +1455,13 @@ textiowrapper_dealloc(textio *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
     self->finalizing = 1;
-    if (_PyIOBase_finalize((PyObject *) self) < 0)
+    if (_PyIOBase_finalize((PyObject *) self) < 0) {
+        // if the object is resurrected, it will be tracked again by
+        // PyObject_CallFinalizerFromDealloc()
+        assert(_PyObject_GC_IS_TRACKED(self));
         return;
+    }
     self->ok = 0;
-    _PyObject_GC_UNTRACK(self);
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)self);
     (void)textiowrapper_clear(self);

Modules/_io/winconsoleio.c

@@ -463,9 +463,12 @@ winconsoleio_dealloc(winconsoleio *self)
 {
     PyTypeObject *tp = Py_TYPE(self);
     self->finalizing = 1;
-    if (_PyIOBase_finalize((PyObject *) self) < 0)
+    if (_PyIOBase_finalize((PyObject *) self) < 0) {
+        // if the object is resurrected, it will be tracked again by
+        // PyObject_CallFinalizerFromDealloc()
+        assert(_PyObject_GC_IS_TRACKED(self));
         return;
-    _PyObject_GC_UNTRACK(self);
+    }
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *) self);
     Py_CLEAR(self->dict);

Objects/bytearrayobject.c

@@ -2440,7 +2440,6 @@ typedef struct {
 static void
 bytearrayiter_dealloc(bytesiterobject *it)
 {
-    _PyObject_GC_UNTRACK(it);
     Py_XDECREF(it->it_seq);
     PyObject_GC_Del(it);
 }

Objects/bytesobject.c

@@ -3223,7 +3223,6 @@ typedef struct {
 static void
 striter_dealloc(striterobject *it)
 {
-    _PyObject_GC_UNTRACK(it);
     Py_XDECREF(it->it_seq);
     PyObject_GC_Del(it);
 }

Objects/cellobject.c

@@ -74,7 +74,6 @@ PyCell_Set(PyObject *op, PyObject *value)
 static void
 cell_dealloc(PyCellObject *op)
 {
-    _PyObject_GC_UNTRACK(op);
     Py_XDECREF(op->ob_ref);
     PyObject_GC_Del(op);
 }

Objects/classobject.c

@@ -237,7 +237,6 @@ method_new_impl(PyTypeObject *type, PyObject *function, PyObject *instance)
 static void
 method_dealloc(PyMethodObject *im)
 {
-    _PyObject_GC_UNTRACK(im);
     if (im->im_weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)im);
     Py_DECREF(im->im_func);
@@ -432,7 +431,6 @@ instancemethod_getattro(PyObject *self, PyObject *name)
 
 static void
 instancemethod_dealloc(PyObject *self) {
-    _PyObject_GC_UNTRACK(self);
     Py_DECREF(PyInstanceMethod_GET_FUNCTION(self));
     PyObject_GC_Del(self);
 }

Objects/descrobject.c

@@ -22,7 +22,6 @@ static void
 descr_dealloc(PyObject *self)
 {
     PyDescrObject *descr = (PyDescrObject *)self;
-    _PyObject_GC_UNTRACK(descr);
     Py_XDECREF(descr->d_type);
     Py_XDECREF(descr->d_name);
     Py_XDECREF(descr->d_qualname);
@@ -1187,7 +1186,6 @@ static void
 mappingproxy_dealloc(PyObject *self)
 {
     mappingproxyobject *pp = (mappingproxyobject *)self;
-    _PyObject_GC_UNTRACK(pp);
     Py_DECREF(pp->mapping);
     PyObject_GC_Del(pp);
 }
@@ -1633,7 +1631,6 @@ property_dealloc(PyObject *self)
 {
     propertyobject *gs = (propertyobject *)self;
 
-    _PyObject_GC_UNTRACK(self);
     Py_XDECREF(gs->prop_get);
     Py_XDECREF(gs->prop_set);
     Py_XDECREF(gs->prop_del);

Objects/dictobject.c

@@ -5015,8 +5015,6 @@ static void
 dictiter_dealloc(PyObject *self)
 {
     dictiterobject *di = (dictiterobject *)self;
-    /* bpo-31095: UnTrack is needed before calling any callbacks */
-    _PyObject_GC_UNTRACK(di);
     Py_XDECREF(di->di_dict);
     Py_XDECREF(di->di_result);
     PyObject_GC_Del(di);
@@ -5815,8 +5813,6 @@ static void
 dictview_dealloc(PyObject *self)
 {
     _PyDictViewObject *dv = (_PyDictViewObject *)self;
-    /* bpo-31095: UnTrack is needed before calling any callbacks */
-    _PyObject_GC_UNTRACK(dv);
     Py_XDECREF(dv->dv_dict);
     PyObject_GC_Del(dv);
 }

Objects/exceptions.c

@@ -698,7 +698,6 @@ SystemExit_clear(PySystemExitObject *self)
 static void
 SystemExit_dealloc(PySystemExitObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     SystemExit_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -887,7 +886,6 @@ BaseExceptionGroup_clear(PyBaseExceptionGroupObject *self)
 static void
 BaseExceptionGroup_dealloc(PyBaseExceptionGroupObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     BaseExceptionGroup_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -1605,7 +1603,6 @@ ImportError_clear(PyImportErrorObject *self)
 static void
 ImportError_dealloc(PyImportErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     ImportError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -1995,7 +1992,6 @@ OSError_clear(PyOSErrorObject *self)
 static void
 OSError_dealloc(PyOSErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     OSError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -2266,7 +2262,6 @@ NameError_clear(PyNameErrorObject *self)
 static void
 NameError_dealloc(PyNameErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     NameError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -2342,7 +2337,6 @@ AttributeError_clear(PyAttributeErrorObject *self)
 static void
 AttributeError_dealloc(PyAttributeErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     AttributeError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -2480,7 +2474,6 @@ SyntaxError_clear(PySyntaxErrorObject *self)
 static void
 SyntaxError_dealloc(PySyntaxErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     SyntaxError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -2930,7 +2923,6 @@ UnicodeError_clear(PyUnicodeErrorObject *self)
 static void
 UnicodeError_dealloc(PyUnicodeErrorObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
     UnicodeError_clear(self);
     Py_TYPE(self)->tp_free((PyObject *)self);
 }
@@ -3389,8 +3381,6 @@ _PyErr_NoMemory(PyThreadState *tstate)
 static void
 MemoryError_dealloc(PyBaseExceptionObject *self)
 {
-    _PyObject_GC_UNTRACK(self);
-
     BaseException_clear(self);
 
     /* If this is a subclass of MemoryError, we don't need to

Objects/frameobject.c

@@ -1637,10 +1637,6 @@ frame_dealloc(PyFrameObject *f)
     /* It is the responsibility of the owning generator/coroutine
      * to have cleared the generator pointer */
 
-    if (_PyObject_GC_IS_TRACKED(f)) {
-        _PyObject_GC_UNTRACK(f);
-    }
-
     Py_TRASHCAN_BEGIN(f, frame_dealloc);
     /* GH-106092: If f->f_frame was on the stack and we reached the maximum
      * nesting depth for deallocations, the trashcan may have delayed this