Skip to content

bpo-35983: improve and test old trashcan macros #12607

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.

Sign up for GitHub

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
jdemeyer wants to merge 1 commit into from
Closed

bpo-35983: improve and test old trashcan macros #12607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions Include/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,10 @@ CAUTION: Never return from the middle of the body! If the body needs to
call, and goto it. Else the call-depth counter (see below) will stay
above 0 forever, and the trashcan will never get emptied.

CAUTION: Any object using the trashcan must be an untracked GC object.
This is because the implementation of the trashcan reuses the GC data
structure.

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
Expand Down Expand Up @@ -725,9 +729,12 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
#define Py_TRASHCAN_BEGIN(op, dealloc) Py_TRASHCAN_BEGIN_CONDITION(op, \
Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))

/* For backwards compatibility, these macros enable the trashcan
* unconditionally */
#define Py_TRASHCAN_SAFE_BEGIN(op) Py_TRASHCAN_BEGIN_CONDITION(op, 1)
/* Old trashcan macros which don't take a "dealloc" argument. To be safe, we
* enable the trashcan only for classes inheriting directly from "object",
* where the problem of a subclass also using the trashcan does not appear.
* This is probably the most common case anyway. */
#define Py_TRASHCAN_SAFE_BEGIN(op) Py_TRASHCAN_BEGIN_CONDITION(op, \
Py_TYPE(op)->tp_base == &PyBaseObject_Type)
#define Py_TRASHCAN_SAFE_END(op) Py_TRASHCAN_END


Expand Down
9 changes: 9 additions & 0 deletions Lib/test/test_capi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,15 @@ def test_trashcan_subclass(self):
for i in range(1000):
L = MyList((L,))

def test_trashcan_compat(self):
# bpo-35983: Check that the trashcan works with the
# backwards-compatibility macros
# Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END
from _testcapi import SingleContainer
L = None
for i in range(2**20):
L = SingleContainer(L)

def test_trashcan_python_class1(self):
self.do_test_trashcan_python_class(list)

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_descr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4334,7 +4334,7 @@ class Oops(object):
o.whatever = Provoker(o)
del o

def test_wrapper_segfault(self):
def test_wrapper_trashcan(self):
# SF 927248: deeply nested wrappers could cause stack overflow
f = lambda:None
for i in range(1000000):
Expand Down
84 changes: 84 additions & 0 deletions Modules/_testcapimodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5520,6 +5520,85 @@ static PyTypeObject MyList_Type = {
MyList_new, /* tp_new */
};

/* Test bpo-35983: trashcan backwards compatibility macros */

typedef struct {
PyObject_HEAD
PyObject *item;
} SingleContainerObject;

static PyObject *
SingleContainer_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
{
PyObject *obj;
if (!_PyArg_NoKeywords("SingleContainer", kwargs)) {
return NULL;
}
if (!PyArg_UnpackTuple(args, "SingleContainer", 1, 1, &obj)) {
return NULL;
}
SingleContainerObject *op = PyObject_GC_New(SingleContainerObject, type);
if (op) {
Py_INCREF(obj);
op->item = obj;
}
return (PyObject *)op;
}

void
SingleContainer_dealloc(SingleContainerObject* op)
{
/* Intentionally use the old
* Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END macros */
PyObject_GC_UnTrack(op);
Py_TRASHCAN_SAFE_BEGIN(op);
Py_DECREF(op->item);
PyObject_GC_Del(op);
Py_TRASHCAN_SAFE_END(op);
}

static PyTypeObject SingleContainer_Type = {
PyVarObject_HEAD_INIT(NULL, 0)
"SingleContainer",
sizeof(SingleContainerObject),
0,
(destructor)SingleContainer_dealloc, /* tp_dealloc */
0, /* tp_print */
0, /* tp_getattr */
0, /* tp_setattr */
0, /* tp_reserved */
0, /* tp_repr */
0, /* tp_as_number */
0, /* tp_as_sequence */
0, /* tp_as_mapping */
0, /* tp_hash */
0, /* tp_call */
0, /* tp_str */
0, /* tp_getattro */
0, /* tp_setattro */
0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
Py_TPFLAGS_HAVE_GC, /* tp_flags */
0, /* tp_doc */
0, /* tp_traverse */
0, /* tp_clear */
0, /* tp_richcompare */
0, /* tp_weaklistoffset */
0, /* tp_iter */
0, /* tp_iternext */
0, /* tp_methods */
0, /* tp_members */
0, /* tp_getset */
0, /* tp_base */
0, /* tp_dict */
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
0, /* tp_init */
0, /* tp_alloc */
SingleContainer_new /* tp_new */
};


/* Test PEP 560 */

Expand Down Expand Up @@ -5640,6 +5719,11 @@ PyInit__testcapi(void)
Py_INCREF(&MyList_Type);
PyModule_AddObject(m, "MyList", (PyObject *)&MyList_Type);

if (PyType_Ready(&SingleContainer_Type) < 0)
return NULL;
Py_INCREF(&SingleContainer_Type);
PyModule_AddObject(m, "SingleContainer", (PyObject *)&SingleContainer_Type);

if (PyType_Ready(&GenericAlias_Type) < 0)
return NULL;
Py_INCREF(&GenericAlias_Type);
Expand Down