Skip to content

Commit a7b4fef

Browse files
jdemeyerjdemeyer
committed
bpo-35983: improve and test old trashcan macros
1 parent 5c86e23 commit a7b4fef

File tree

4 files changed

+104
-4
lines changed

4 files changed

+104
-4
lines changed

‎Include/object.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,10 @@ CAUTION: Never return from the middle of the body! If the body needs to
674674
call, and goto it. Else the call-depth counter (see below) will stay
675675
above 0 forever, and the trashcan will never get emptied.
676676
677+
CAUTION: Any object using the trashcan must be an untracked GC object.
678+
This is because the implementation of the trashcan reuses the GC data
679+
structure.
680+
677681
How it works: The BEGIN macro increments a call-depth counter. So long
678682
as this counter is small, the body of the deallocator is run directly without
679683
further ado. But if the counter gets large, it instead adds p to a list of
@@ -727,9 +731,12 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
727731
#define Py_TRASHCAN_BEGIN(op, dealloc) Py_TRASHCAN_BEGIN_CONDITION(op, \
728732
Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))
729733

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

735742

‎Lib/test/test_capi.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,15 @@ def test_trashcan_subclass(self):
341341
for i in range(1000):
342342
L = MyList((L,))
343343

344+
def test_trashcan_compat(self):
345+
# bpo-35983: Check that the trashcan works with the
346+
# backwards-compatibility macros
347+
# Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END
348+
from _testcapi import SingleContainer
349+
L = None
350+
for i in range(2**20):
351+
L = SingleContainer(L)
352+
344353
def test_trashcan_python_class(self):
345354
# Check that the trashcan mechanism works properly for a Python
346355
# subclass of a class using the trashcan (list in this test)

‎Lib/test/test_descr.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4307,7 +4307,7 @@ class Oops(object):
43074307
o.whatever = Provoker(o)
43084308
del o
43094309

4310-
def test_wrapper_segfault(self):
4310+
def test_wrapper_trashcan(self):
43114311
# SF 927248: deeply nested wrappers could cause stack overflow
43124312
f = lambda:None
43134313
for i in range(1000000):

‎Modules/_testcapimodule.c‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5399,6 +5399,85 @@ static PyTypeObject MyList_Type = {
53995399
MyList_new, /* tp_new */
54005400
};
54015401

5402+
/* Test bpo-35983: trashcan backwards compatibility macros */
5403+
5404+
typedef struct {
5405+
PyObject_HEAD
5406+
PyObject *item;
5407+
} SingleContainerObject;
5408+
5409+
static PyObject *
5410+
SingleContainer_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
5411+
{
5412+
PyObject *obj;
5413+
if (!_PyArg_NoKeywords("SingleContainer", kwargs)) {
5414+
return NULL;
5415+
}
5416+
if (!PyArg_UnpackTuple(args, "SingleContainer", 1, 1, &obj)) {
5417+
return NULL;
5418+
}
5419+
SingleContainerObject *op = PyObject_GC_New(SingleContainerObject, type);
5420+
if (op) {
5421+
Py_INCREF(obj);
5422+
op->item = obj;
5423+
}
5424+
return (PyObject *)op;
5425+
}
5426+
5427+
void
5428+
SingleContainer_dealloc(SingleContainerObject* op)
5429+
{
5430+
/* Intentionally use the old
5431+
* Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END macros */
5432+
PyObject_GC_UnTrack(op);
5433+
Py_TRASHCAN_SAFE_BEGIN(op);
5434+
Py_DECREF(op->item);
5435+
PyObject_GC_Del(op);
5436+
Py_TRASHCAN_SAFE_END(op);
5437+
}
5438+
5439+
static PyTypeObject SingleContainer_Type = {
5440+
PyVarObject_HEAD_INIT(NULL, 0)
5441+
"SingleContainer",
5442+
sizeof(SingleContainerObject),
5443+
0,
5444+
(destructor)SingleContainer_dealloc, /* tp_dealloc */
5445+
0, /* tp_print */
5446+
0, /* tp_getattr */
5447+
0, /* tp_setattr */
5448+
0, /* tp_reserved */
5449+
0, /* tp_repr */
5450+
0, /* tp_as_number */
5451+
0, /* tp_as_sequence */
5452+
0, /* tp_as_mapping */
5453+
0, /* tp_hash */
5454+
0, /* tp_call */
5455+
0, /* tp_str */
5456+
0, /* tp_getattro */
5457+
0, /* tp_setattro */
5458+
0, /* tp_as_buffer */
5459+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
5460+
Py_TPFLAGS_HAVE_GC, /* tp_flags */
5461+
0, /* tp_doc */
5462+
0, /* tp_traverse */
5463+
0, /* tp_clear */
5464+
0, /* tp_richcompare */
5465+
0, /* tp_weaklistoffset */
5466+
0, /* tp_iter */
5467+
0, /* tp_iternext */
5468+
0, /* tp_methods */
5469+
0, /* tp_members */
5470+
0, /* tp_getset */
5471+
0, /* tp_base */
5472+
0, /* tp_dict */
5473+
0, /* tp_descr_get */
5474+
0, /* tp_descr_set */
5475+
0, /* tp_dictoffset */
5476+
0, /* tp_init */
5477+
0, /* tp_alloc */
5478+
SingleContainer_new /* tp_new */
5479+
};
5480+
54025481

54035482
/* Test PEP 560 */
54045483

@@ -5519,6 +5598,11 @@ PyInit__testcapi(void)
55195598
Py_INCREF(&MyList_Type);
55205599
PyModule_AddObject(m, "MyList", (PyObject *)&MyList_Type);
55215600

5601+
if (PyType_Ready(&SingleContainer_Type) < 0)
5602+
return NULL;
5603+
Py_INCREF(&SingleContainer_Type);
5604+
PyModule_AddObject(m, "SingleContainer", (PyObject *)&SingleContainer_Type);
5605+
55225606
if (PyType_Ready(&GenericAlias_Type) < 0)
55235607
return NULL;
55245608
Py_INCREF(&GenericAlias_Type);

0 commit comments

Comments
 (0)