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