Skip to content

bpo-44897: WIP: Integrate trashcan into _Py_Dealloc #27738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 6 additions & 74 deletions Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,83 +456,15 @@ PyAPI_FUNC(int) _PyObject_CheckConsistency(
int check_content);


/* 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)/(_PyTrash_UNWIND_LEVEL-1) pieces,
with the call stack never exceeding a depth of _PyTrash_UNWIND_LEVEL.

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().
*/

/* Forward declarations for PyThreadState */
struct _ts;

/* Python 3.9 private API, invoked by the macros below. */
PyAPI_FUNC(int) _PyTrash_begin(struct _ts *tstate, PyObject *op);
PyAPI_FUNC(void) _PyTrash_end(struct _ts *tstate);
/* Python 3.10 private API, invoked by the Py_TRASHCAN_BEGIN(). */
PyAPI_FUNC(int) _PyTrash_cond(PyObject *op, destructor dealloc);

#define Py_TRASHCAN_BEGIN_CONDITION(op, cond) \
do { \
PyThreadState *_tstate = NULL; \
/* If "cond" is false, then _tstate remains NULL and the deallocator \
* is run normally without involving the trashcan */ \
if (cond) { \
_tstate = PyThreadState_Get(); \
if (_PyTrash_begin(_tstate, _PyObject_CAST(op))) { \
break; \
} \
}
/* The body of the deallocator is here. */
/* 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 {
#define Py_TRASHCAN_END \
if (_tstate) { \
_PyTrash_end(_tstate); \
} \
assert(1); \
} while (0);

#define Py_TRASHCAN_BEGIN(op, dealloc) \
Py_TRASHCAN_BEGIN_CONDITION(op, \
_PyTrash_cond(_PyObject_CAST(op), (destructor)dealloc))

/* For backwards compatibility, these macros enable the trashcan
* unconditionally */
Expand Down
5 changes: 0 additions & 5 deletions Modules/_elementtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -664,10 +664,6 @@ element_gc_clear(ElementObject *self)
static void
element_dealloc(ElementObject* self)
{
/* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(self);
Py_TRASHCAN_BEGIN(self, element_dealloc)

if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);

Expand All @@ -677,7 +673,6 @@ element_dealloc(ElementObject* self)

RELEASE(sizeof(ElementObject), "destroy element");
Py_TYPE(self)->tp_free((PyObject *)self);
Py_TRASHCAN_END
}

/* -------------------------------------------------------------------- */
Expand Down
2 changes: 0 additions & 2 deletions Modules/_io/bufferedio.c
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ buffered_dealloc(buffered *self)
self->finalizing = 1;
if (_PyIOBase_finalize((PyObject *) self) < 0)
return;
_PyObject_GC_UNTRACK(self);
self->ok = 0;
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)self);
Expand Down Expand Up @@ -2144,7 +2143,6 @@ bufferedrwpair_clear(rwpair *self)
static void
bufferedrwpair_dealloc(rwpair *self)
{
_PyObject_GC_UNTRACK(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)self);
Py_CLEAR(self->reader);
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/bytesio.c
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,6 @@ bytesio_setstate(bytesio *self, PyObject *state)
static void
bytesio_dealloc(bytesio *self)
{
_PyObject_GC_UNTRACK(self);
if (self->exports > 0) {
PyErr_SetString(PyExc_SystemError,
"deallocated BytesIO object has exported buffers");
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/fileio.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,6 @@ fileio_dealloc(fileio *self)
self->finalizing = 1;
if (_PyIOBase_finalize((PyObject *) self) < 0)
return;
_PyObject_GC_UNTRACK(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);
Py_CLEAR(self->dict);
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/iobase.c
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,6 @@ iobase_dealloc(iobase *self)
}
return;
}
_PyObject_GC_UNTRACK(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);
Py_CLEAR(self->dict);
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/stringio.c
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,6 @@ stringio_clear(stringio *self)
static void
stringio_dealloc(stringio *self)
{
_PyObject_GC_UNTRACK(self);
self->ok = 0;
if (self->buf) {
PyMem_Free(self->buf);
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/textio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,6 @@ textiowrapper_dealloc(textio *self)
if (_PyIOBase_finalize((PyObject *) self) < 0)
return;
self->ok = 0;
_PyObject_GC_UNTRACK(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)self);
textiowrapper_clear(self);
Expand Down
1 change: 0 additions & 1 deletion Modules/_io/winconsoleio.c
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ winconsoleio_dealloc(winconsoleio *self)
self->finalizing = 1;
if (_PyIOBase_finalize((PyObject *) self) < 0)
return;
_PyObject_GC_UNTRACK(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);
Py_CLEAR(self->dict);
Expand Down
1 change: 0 additions & 1 deletion Objects/bytearrayobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2371,7 +2371,6 @@ typedef struct {
static void
bytearrayiter_dealloc(bytesiterobject *it)
{
_PyObject_GC_UNTRACK(it);
Py_XDECREF(it->it_seq);
PyObject_GC_Del(it);
}
Expand Down
1 change: 0 additions & 1 deletion Objects/bytesobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3095,7 +3095,6 @@ typedef struct {
static void
striter_dealloc(striterobject *it)
{
_PyObject_GC_UNTRACK(it);
Py_XDECREF(it->it_seq);
PyObject_GC_Del(it);
}
Expand Down
1 change: 0 additions & 1 deletion Objects/cellobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ PyCell_Set(PyObject *op, PyObject *obj)
static void
cell_dealloc(PyCellObject *op)
{
_PyObject_GC_UNTRACK(op);
Py_XDECREF(op->ob_ref);
PyObject_GC_Del(op);
}
Expand Down
2 changes: 0 additions & 2 deletions Objects/classobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ method_new(PyTypeObject* type, PyObject* args, PyObject *kw)
static void
method_dealloc(PyMethodObject *im)
{
_PyObject_GC_UNTRACK(im);
if (im->im_weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)im);
Py_DECREF(im->im_func);
Expand Down Expand Up @@ -448,7 +447,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);
}
Expand Down
6 changes: 0 additions & 6 deletions Objects/descrobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class property "propertyobject *" "&PyProperty_Type"
static void
descr_dealloc(PyDescrObject *descr)
{
_PyObject_GC_UNTRACK(descr);
Py_XDECREF(descr->d_type);
Py_XDECREF(descr->d_name);
Py_XDECREF(descr->d_qualname);
Expand Down Expand Up @@ -1154,7 +1153,6 @@ static PyMethodDef mappingproxy_methods[] = {
static void
mappingproxy_dealloc(mappingproxyobject *pp)
{
_PyObject_GC_UNTRACK(pp);
Py_DECREF(pp->mapping);
PyObject_GC_Del(pp);
}
Expand Down Expand Up @@ -1265,12 +1263,9 @@ typedef struct {
static void
wrapper_dealloc(wrapperobject *wp)
{
PyObject_GC_UnTrack(wp);
Py_TRASHCAN_BEGIN(wp, wrapper_dealloc)
Py_XDECREF(wp->descr);
Py_XDECREF(wp->self);
PyObject_GC_Del(wp);
Py_TRASHCAN_END
}

static PyObject *
Expand Down Expand Up @@ -1577,7 +1572,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);
Expand Down
8 changes: 0 additions & 8 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1928,9 +1928,6 @@ dict_dealloc(PyDictObject *mp)
PyDictKeysObject *keys = mp->ma_keys;
Py_ssize_t i, n;

/* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(mp);
Py_TRASHCAN_BEGIN(mp, dict_dealloc)
if (values != NULL) {
if (values != empty_values) {
for (i = 0, n = mp->ma_keys->dk_nentries; i < n; i++) {
Expand All @@ -1955,7 +1952,6 @@ dict_dealloc(PyDictObject *mp)
else {
Py_TYPE(mp)->tp_free((PyObject *)mp);
}
Py_TRASHCAN_END
}


Expand Down Expand Up @@ -3570,8 +3566,6 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
static void
dictiter_dealloc(dictiterobject *di)
{
/* 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);
Expand Down Expand Up @@ -4066,8 +4060,6 @@ PyTypeObject PyDictRevIterValue_Type = {
static void
dictview_dealloc(_PyDictViewObject *dv)
{
/* bpo-31095: UnTrack is needed before calling any callbacks */
_PyObject_GC_UNTRACK(dv);
Py_XDECREF(dv->dv_dict);
PyObject_GC_Del(dv);
}
Expand Down
11 changes: 0 additions & 11 deletions Objects/exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ BaseException_clear(PyBaseExceptionObject *self)
static void
BaseException_dealloc(PyBaseExceptionObject *self)
{
_PyObject_GC_UNTRACK(self);
BaseException_clear(self);
Py_TYPE(self)->tp_free((PyObject *)self);
}
Expand Down Expand Up @@ -535,7 +534,6 @@ StopIteration_clear(PyStopIterationObject *self)
static void
StopIteration_dealloc(PyStopIterationObject *self)
{
_PyObject_GC_UNTRACK(self);
StopIteration_clear(self);
Py_TYPE(self)->tp_free((PyObject *)self);
}
Expand Down Expand Up @@ -602,7 +600,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);
}
Expand Down Expand Up @@ -684,7 +681,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);
}
Expand Down Expand Up @@ -1087,7 +1083,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);
}
Expand Down Expand Up @@ -1364,7 +1359,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);
}
Expand Down Expand Up @@ -1443,7 +1437,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);
}
Expand Down Expand Up @@ -1538,7 +1531,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);
}
Expand Down Expand Up @@ -1988,7 +1980,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);
}
Expand Down Expand Up @@ -2429,8 +2420,6 @@ MemoryError_dealloc(PyBaseExceptionObject *self)
return;
}

_PyObject_GC_UNTRACK(self);

struct _Py_exc_state *state = get_exc_state();
if (state->memerrors_numfree >= MEMERRORS_SAVE) {
Py_TYPE(self)->tp_free((PyObject *)self);
Expand Down
6 changes: 0 additions & 6 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -615,11 +615,6 @@ static PyGetSetDef frame_getsetlist[] = {
static void _Py_HOT_FUNCTION
frame_dealloc(PyFrameObject *f)
{
if (_PyObject_GC_IS_TRACKED(f)) {
_PyObject_GC_UNTRACK(f);
}

Py_TRASHCAN_BEGIN(f, frame_dealloc);
PyCodeObject *co = NULL;

/* Kill all local variables including specials, if we own them */
Expand Down Expand Up @@ -659,7 +654,6 @@ frame_dealloc(PyFrameObject *f)
}

Py_XDECREF(co);
Py_TRASHCAN_END;
}

static int
Expand Down
Loading