pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/python/cpython/commit/351c67416ba4451eb3928fa0b2e933c2f25df1a3

aa60c69660fa.css" /> bpo-35983: skip trashcan for subclasses (GH-11841) · python/cpython@351c674 · GitHub
Skip to content

Commit 351c674

Browse files
jdemeyerpitrou
jdemeyer
authored and
pitrou
committed
bpo-35983: skip trashcan for subclasses (GH-11841)
Add new trashcan macros to deal with a double deallocation that could occur when the `tp_dealloc` of a subclass calls the `tp_dealloc` of a base class and that base class uses the trashcan mechanism. Patch by Jeroen Demeyer.
1 parent a2fedd8 commit 351c674

15 files changed

Lines changed: 189 additions & 121 deletions

File tree

‎Include/object.h‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -649,26 +649,26 @@ times.
649649
650650
When deallocating a container object, it's possible to trigger an unbounded
651651
chain of deallocations, as each Py_DECREF in turn drops the refcount on "the
652-
next" object in the chain to 0. This can easily lead to stack faults, and
652+
next" object in the chain to 0. This can easily lead to stack overflows,
653653
especially in threads (which typically have less stack space to work with).
654654
655-
A container object that participates in cyclic gc can avoid this by
656-
bracketing the body of its tp_dealloc function with a pair of macros:
655+
A container object can avoid this by bracketing the body of its tp_dealloc
656+
function with a pair of macros:
657657
658658
static void
659659
mytype_dealloc(mytype *p)
660660
{
661661
... declarations go here ...
662662
663663
PyObject_GC_UnTrack(p); // must untrack first
664-
Py_TRASHCAN_SAFE_BEGIN(p)
664+
Py_TRASHCAN_BEGIN(p, mytype_dealloc)
665665
... The body of the deallocator goes here, including all calls ...
666666
... to Py_DECREF on contained objects. ...
667-
Py_TRASHCAN_SAFE_END(p)
667+
Py_TRASHCAN_END // there should be no code after this
668668
}
669669
670670
CAUTION: Never return from the middle of the body! If the body needs to
671-
"get out early", put a label immediately before the Py_TRASHCAN_SAFE_END
671+
"get out early", put a label immediately before the Py_TRASHCAN_END
672672
call, and goto it. Else the call-depth counter (see below) will stay
673673
above 0 forever, and the trashcan will never get emptied.
674674
@@ -684,6 +684,12 @@ notices this, and calls another routine to deallocate all the objects that
684684
may have been added to the list of deferred deallocations. In effect, a
685685
chain of N deallocations is broken into (N-1)/(PyTrash_UNWIND_LEVEL-1) pieces,
686686
with the call stack never exceeding a depth of PyTrash_UNWIND_LEVEL.
687+
688+
Since the tp_dealloc of a subclass typically calls the tp_dealloc of the base
689+
class, we need to ensure that the trashcan is only triggered on the tp_dealloc
690+
of the actual class being deallocated. Otherwise we might end up with a
691+
partially-deallocated object. To check this, the tp_dealloc function must be
692+
passed as second argument to Py_TRASHCAN_BEGIN().
687693
*/
688694

689695
/* The new thread-safe private API, invoked by the macros below. */
@@ -692,21 +698,38 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
692698

693699
#define PyTrash_UNWIND_LEVEL 50
694700

695-
#define Py_TRASHCAN_SAFE_BEGIN(op) \
701+
#define Py_TRASHCAN_BEGIN_CONDITION(op, cond) \
696702
do { \
697-
PyThreadState *_tstate = PyThreadState_GET(); \
698-
if (_tstate->trash_delete_nesting < PyTrash_UNWIND_LEVEL) { \
699-
++_tstate->trash_delete_nesting;
700-
/* The body of the deallocator is here. */
701-
#define Py_TRASHCAN_SAFE_END(op) \
703+
PyThreadState *_tstate = NULL; \
704+
/* If "cond" is false, then _tstate remains NULL and the deallocator \
705+
* is run normally without involving the trashcan */ \
706+
if (cond) { \
707+
_tstate = PyThreadState_GET(); \
708+
if (_tstate->trash_delete_nesting >= PyTrash_UNWIND_LEVEL) { \
709+
/* Store the object (to be deallocated later) and jump past \
710+
* Py_TRASHCAN_END, skipping the body of the deallocator */ \
711+
_PyTrash_thread_deposit_object(_PyObject_CAST(op)); \
712+
break; \
713+
} \
714+
++_tstate->trash_delete_nesting; \
715+
}
716+
/* The body of the deallocator is here. */
717+
#define Py_TRASHCAN_END \
718+
if (_tstate) { \
702719
--_tstate->trash_delete_nesting; \
703720
if (_tstate->trash_delete_later && _tstate->trash_delete_nesting <= 0) \
704721
_PyTrash_thread_destroy_chain(); \
705722
} \
706-
else \
707-
_PyTrash_thread_deposit_object(_PyObject_CAST(op)); \
708723
} while (0);
709724

725+
#define Py_TRASHCAN_BEGIN(op, dealloc) Py_TRASHCAN_BEGIN_CONDITION(op, \
726+
Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))
727+
728+
/* For backwards compatibility, these macros enable the trashcan
729+
* unconditionally */
730+
#define Py_TRASHCAN_SAFE_BEGIN(op) Py_TRASHCAN_BEGIN_CONDITION(op, 1)
731+
#define Py_TRASHCAN_SAFE_END(op) Py_TRASHCAN_END
732+
710733

711734
#ifndef Py_LIMITED_API
712735
# define Py_CPYTHON_OBJECT_H

‎Lib/test/test_capi.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,49 @@ def test_negative_refcount(self):
333333
br'_Py_NegativeRefcount: Assertion failed: '
334334
br'object has negative ref count')
335335

336+
def test_trashcan_subclass(self):
337+
# bpo-35983: Check that the trashcan mechanism for "list" is NOT
338+
# activated when its tp_dealloc is being called by a subclass
339+
from _testcapi import MyList
340+
L = None
341+
for i in range(1000):
342+
L = MyList((L,))
343+
344+
def test_trashcan_python_class1(self):
345+
self.do_test_trashcan_python_class(list)
346+
347+
def test_trashcan_python_class2(self):
348+
from _testcapi import MyList
349+
self.do_test_trashcan_python_class(MyList)
350+
351+
def do_test_trashcan_python_class(self, base):
352+
# Check that the trashcan mechanism works properly for a Python
353+
# subclass of a class using the trashcan (this specific test assumes
354+
# that the base class "base" behaves like list)
355+
class PyList(base):
356+
# Count the number of PyList instances to verify that there is
357+
# no memory leak
358+
num = 0
359+
def __init__(self, *args):
360+
__class__.num += 1
361+
super().__init__(*args)
362+
def __del__(self):
363+
__class__.num -= 1
364+
365+
for parity in (0, 1):
366+
L = None
367+
# We need in the order of 2**20 iterations here such that a
368+
# typical 8MB stack would overflow without the trashcan.
369+
for i in range(2**20):
370+
L = PyList((L,))
371+
L.attr = i
372+
if parity:
373+
# Add one additional nesting layer
374+
L = (L,)
375+
self.assertGreater(PyList.num, 0)
376+
del L
377+
self.assertEqual(PyList.num, 0)
378+
336379

337380
class TestPendingCalls(unittest.TestCase):
338381

‎Lib/test/test_ordered_dict.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,9 @@ def update(self, *args, **kwds):
459459
self.assertEqual(list(MyOD(items).items()), items)
460460

461461
def test_highly_nested(self):
462-
# Issue 25395: crashes during garbage collection
462+
# Issues 25395 and 35983: test that the trashcan mechanism works
463+
# correctly for OrderedDict: deleting a highly nested OrderDict
464+
# should not crash Python.
463465
OrderedDict = self.OrderedDict
464466
obj = None
465467
for _ in range(1000):
@@ -468,7 +470,9 @@ def test_highly_nested(self):
468470
support.gc_collect()
469471

470472
def test_highly_nested_subclass(self):
471-
# Issue 25395: crashes during garbage collection
473+
# Issues 25395 and 35983: test that the trashcan mechanism works
474+
# correctly for OrderedDict: deleting a highly nested OrderDict
475+
# should not crash Python.
472476
OrderedDict = self.OrderedDict
473477
deleted = []
474478
class MyOD(OrderedDict):

‎Misc/NEWS.d/next/Core and Builtins/2019-02-13-16-47-19.bpo-35983.bNxsXv.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Added new trashcan macros to deal with a double deallocation that could occur
2+
when the `tp_dealloc` of a subclass calls the `tp_dealloc` of a base class
3+
and that base class uses the trashcan mechanism. Patch by Jeroen Demeyer.

‎Modules/_elementtree.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ element_dealloc(ElementObject* self)
669669
{
670670
/* bpo-31095: UnTrack is needed before calling any callbacks */
671671
PyObject_GC_UnTrack(self);
672-
Py_TRASHCAN_SAFE_BEGIN(self)
672+
Py_TRASHCAN_BEGIN(self, element_dealloc)
673673

674674
if (self->weakreflist != NULL)
675675
PyObject_ClearWeakRefs((PyObject *) self);
@@ -680,7 +680,7 @@ element_dealloc(ElementObject* self)
680680

681681
RELEASE(sizeof(ElementObject), "destroy element");
682682
Py_TYPE(self)->tp_free((PyObject *)self);
683-
Py_TRASHCAN_SAFE_END(self)
683+
Py_TRASHCAN_END
684684
}
685685

686686
/* -------------------------------------------------------------------- */

‎Modules/_testcapimodule.c‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5451,6 +5451,76 @@ recurse_infinitely_error_init(PyObject *self, PyObject *args, PyObject *kwds)
54515451
}
54525452

54535453

5454+
/* Test bpo-35983: create a subclass of "list" which checks that instances
5455+
* are not deallocated twice */
5456+
5457+
typedef struct {
5458+
PyListObject list;
5459+
int deallocated;
5460+
} MyListObject;
5461+
5462+
static PyObject *
5463+
MyList_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
5464+
{
5465+
PyObject* op = PyList_Type.tp_new(type, args, kwds);
5466+
((MyListObject*)op)->deallocated = 0;
5467+
return op;
5468+
}
5469+
5470+
void
5471+
MyList_dealloc(MyListObject* op)
5472+
{
5473+
if (op->deallocated) {
5474+
/* We cannot raise exceptions here but we still want the testsuite
5475+
* to fail when we hit this */
5476+
Py_FatalError("MyList instance deallocated twice");
5477+
}
5478+
op->deallocated = 1;
5479+
PyList_Type.tp_dealloc((PyObject *)op);
5480+
}
5481+
5482+
static PyTypeObject MyList_Type = {
5483+
PyVarObject_HEAD_INIT(NULL, 0)
5484+
"MyList",
5485+
sizeof(MyListObject),
5486+
0,
5487+
(destructor)MyList_dealloc, /* tp_dealloc */
5488+
0, /* tp_print */
5489+
0, /* tp_getattr */
5490+
0, /* tp_setattr */
5491+
0, /* tp_reserved */
5492+
0, /* tp_repr */
5493+
0, /* tp_as_number */
5494+
0, /* tp_as_sequence */
5495+
0, /* tp_as_mapping */
5496+
0, /* tp_hash */
5497+
0, /* tp_call */
5498+
0, /* tp_str */
5499+
0, /* tp_getattro */
5500+
0, /* tp_setattro */
5501+
0, /* tp_as_buffer */
5502+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
5503+
0, /* tp_doc */
5504+
0, /* tp_traverse */
5505+
0, /* tp_clear */
5506+
0, /* tp_richcompare */
5507+
0, /* tp_weaklistoffset */
5508+
0, /* tp_iter */
5509+
0, /* tp_iternext */
5510+
0, /* tp_methods */
5511+
0, /* tp_members */
5512+
0, /* tp_getset */
5513+
0, /* &PyList_Type */ /* tp_base */
5514+
0, /* tp_dict */
5515+
0, /* tp_descr_get */
5516+
0, /* tp_descr_set */
5517+
0, /* tp_dictoffset */
5518+
0, /* tp_init */
5519+
0, /* tp_alloc */
5520+
MyList_new, /* tp_new */
5521+
};
5522+
5523+
54545524
/* Test PEP 560 */
54555525

54565526
typedef struct {
@@ -5564,6 +5634,12 @@ PyInit__testcapi(void)
55645634
Py_INCREF(&awaitType);
55655635
PyModule_AddObject(m, "awaitType", (PyObject *)&awaitType);
55665636

5637+
MyList_Type.tp_base = &PyList_Type;
5638+
if (PyType_Ready(&MyList_Type) < 0)
5639+
return NULL;
5640+
Py_INCREF(&MyList_Type);
5641+
PyModule_AddObject(m, "MyList", (PyObject *)&MyList_Type);
5642+
55675643
if (PyType_Ready(&GenericAlias_Type) < 0)
55685644
return NULL;
55695645
Py_INCREF(&GenericAlias_Type);

‎Objects/descrobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,11 +1021,11 @@ static void
10211021
wrapper_dealloc(wrapperobject *wp)
10221022
{
10231023
PyObject_GC_UnTrack(wp);
1024-
Py_TRASHCAN_SAFE_BEGIN(wp)
1024+
Py_TRASHCAN_BEGIN(wp, wrapper_dealloc)
10251025
Py_XDECREF(wp->descr);
10261026
Py_XDECREF(wp->self);
10271027
PyObject_GC_Del(wp);
1028-
Py_TRASHCAN_SAFE_END(wp)
1028+
Py_TRASHCAN_END
10291029
}
10301030

10311031
static PyObject *

‎Objects/dictobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1978,7 +1978,7 @@ dict_dealloc(PyDictObject *mp)
19781978

19791979
/* bpo-31095: UnTrack is needed before calling any callbacks */
19801980
PyObject_GC_UnTrack(mp);
1981-
Py_TRASHCAN_SAFE_BEGIN(mp)
1981+
Py_TRASHCAN_BEGIN(mp, dict_dealloc)
19821982
if (values != NULL) {
19831983
if (values != empty_values) {
19841984
for (i = 0, n = mp->ma_keys->dk_nentries; i < n; i++) {
@@ -1996,7 +1996,7 @@ dict_dealloc(PyDictObject *mp)
19961996
free_list[numfree++] = mp;
19971997
else
19981998
Py_TYPE(mp)->tp_free((PyObject *)mp);
1999-
Py_TRASHCAN_SAFE_END(mp)
1999+
Py_TRASHCAN_END
20002000
}
20012001

20022002

‎Objects/listobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ list_dealloc(PyListObject *op)
361361
{
362362
Py_ssize_t i;
363363
PyObject_GC_UnTrack(op);
364-
Py_TRASHCAN_SAFE_BEGIN(op)
364+
Py_TRASHCAN_BEGIN(op, list_dealloc)
365365
if (op->ob_item != NULL) {
366366
/* Do it backwards, for Christian Tismer.
367367
There's a simple test case where somehow this reduces
@@ -377,7 +377,7 @@ list_dealloc(PyListObject *op)
377377
free_list[numfree++] = op;
378378
else
379379
Py_TYPE(op)->tp_free((PyObject *)op);
380-
Py_TRASHCAN_SAFE_END(op)
380+
Py_TRASHCAN_END
381381
}
382382

383383
static PyObject *

‎Objects/odictobject.c‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,28 +1356,17 @@ static PyGetSetDef odict_getset[] = {
13561356
static void
13571357
odict_dealloc(PyODictObject *self)
13581358
{
1359-
PyThreadState *tstate = _PyThreadState_GET();
1360-
13611359
PyObject_GC_UnTrack(self);
1362-
Py_TRASHCAN_SAFE_BEGIN(self)
1360+
Py_TRASHCAN_BEGIN(self, odict_dealloc)
13631361

13641362
Py_XDECREF(self->od_inst_dict);
13651363
if (self->od_weakreflist != NULL)
13661364
PyObject_ClearWeakRefs((PyObject *)self);
13671365

13681366
_odict_clear_nodes(self);
1369-
1370-
/* Call the base tp_dealloc(). Since it too uses the trashcan mechanism,
1371-
* temporarily decrement trash_delete_nesting to prevent triggering it
1372-
* and putting the partially deallocated object on the trashcan's
1373-
* to-be-deleted-later list.
1374-
*/
1375-
--tstate->trash_delete_nesting;
1376-
assert(_tstate->trash_delete_nesting < PyTrash_UNWIND_LEVEL);
13771367
PyDict_Type.tp_dealloc((PyObject *)self);
1378-
++tstate->trash_delete_nesting;
13791368

1380-
Py_TRASHCAN_SAFE_END(self)
1369+
Py_TRASHCAN_END
13811370
}
13821371

13831372
/* tp_repr */

0 commit comments

Comments
 (0)
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy