Skip to content

Commit 236e1bd

Browse files
mandolaerikmandolaerik
committed
Revert "pythonGH-124715: Move trashcan mechanism into Py_Dealloc (pythonGH-132280)"
This reverts commit 44e4c47.
1 parent a47da5f commit 236e1bd

25 files changed

Lines changed: 213 additions & 78 deletions

‎Include/cpython/object.h‎

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,14 +429,81 @@ PyAPI_FUNC(void) _Py_NO_RETURN _PyObject_AssertFailed(
429429
const char *function);
430430

431431

432+
/* Trashcan mechanism, thanks to Christian Tismer.
433+
434+
When deallocating a container object, it's possible to trigger an unbounded
435+
chain of deallocations, as each Py_DECREF in turn drops the refcount on "the
436+
next" object in the chain to 0. This can easily lead to stack overflows,
437+
especially in threads (which typically have less stack space to work with).
438+
439+
A container object can avoid this by bracketing the body of its tp_dealloc
440+
function with a pair of macros:
441+
442+
static void
443+
mytype_dealloc(mytype *p)
444+
{
445+
... declarations go here ...
446+
447+
PyObject_GC_UnTrack(p); // must untrack first
448+
Py_TRASHCAN_BEGIN(p, mytype_dealloc)
449+
... The body of the deallocator goes here, including all calls ...
450+
... to Py_DECREF on contained objects. ...
451+
Py_TRASHCAN_END // there should be no code after this
452+
}
453+
454+
CAUTION: Never return from the middle of the body! If the body needs to
455+
"get out early", put a label immediately before the Py_TRASHCAN_END
456+
call, and goto it. Else the call-depth counter (see below) will stay
457+
above 0 forever, and the trashcan will never get emptied.
458+
459+
How it works: The BEGIN macro increments a call-depth counter. So long
460+
as this counter is small, the body of the deallocator is run directly without
461+
further ado. But if the counter gets large, it instead adds p to a list of
462+
objects to be deallocated later, skips the body of the deallocator, and
463+
resumes execution after the END macro. The tp_dealloc routine then returns
464+
without deallocating anything (and so unbounded call-stack depth is avoided).
465+
466+
When the call stack finishes unwinding again, code generated by the END macro
467+
notices this, and calls another routine to deallocate all the objects that
468+
may have been added to the list of deferred deallocations. In effect, a
469+
chain of N deallocations is broken into (N-1)/(Py_TRASHCAN_HEADROOM-1) pieces,
470+
with the call stack never exceeding a depth of Py_TRASHCAN_HEADROOM.
471+
472+
Since the tp_dealloc of a subclass typically calls the tp_dealloc of the base
473+
class, we need to ensure that the trashcan is only triggered on the tp_dealloc
474+
of the actual class being deallocated. Otherwise we might end up with a
475+
partially-deallocated object. To check this, the tp_dealloc function must be
476+
passed as second argument to Py_TRASHCAN_BEGIN().
477+
*/
478+
479+
432480
PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op);
433481
PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(PyThreadState *tstate);
434482

483+
484+
/* Python 3.10 private API, invoked by the Py_TRASHCAN_BEGIN(). */
485+
486+
/* To avoid raising recursion errors during dealloc trigger trashcan before we reach
487+
* recursion limit. To avoid trashing, we don't attempt to empty the trashcan until
488+
* we have headroom above the trigger limit */
489+
#define Py_TRASHCAN_HEADROOM 50
490+
491+
/* Helper function for Py_TRASHCAN_BEGIN */
435492
PyAPI_FUNC(int) _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count);
436493

437-
/* For backwards compatibility with the old trashcan mechanism */
438-
#define Py_TRASHCAN_BEGIN(op, dealloc)
439-
#define Py_TRASHCAN_END
494+
#define Py_TRASHCAN_BEGIN(op, dealloc) \
495+
do { \
496+
PyThreadState *tstate = PyThreadState_Get(); \
497+
if (_Py_ReachedRecursionLimitWithMargin(tstate, 2) && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
498+
_PyTrash_thread_deposit_object(tstate, (PyObject *)op); \
499+
break; \
500+
}
501+
/* The body of the deallocator is here. */
502+
#define Py_TRASHCAN_END \
503+
if (tstate->delete_later && !_Py_ReachedRecursionLimitWithMargin(tstate, 4)) { \
504+
_PyTrash_thread_destroy_chain(tstate); \
505+
} \
506+
} while (0);
440507

441508

442509
PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj);

‎Include/internal/pycore_ceval.h‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,25 @@ extern void _PyEval_DeactivateOpCache(void);
198198

199199
/* --- _Py_EnterRecursiveCall() ----------------------------------------- */
200200

201+
#if !_Py__has_builtin(__builtin_frame_address) && !defined(_MSC_VER)
202+
static uintptr_t return_pointer_as_int(char* p) {
203+
return (uintptr_t)p;
204+
}
205+
#endif
206+
207+
static inline uintptr_t
208+
_Py_get_machine_stack_pointer(void) {
209+
#if _Py__has_builtin(__builtin_frame_address)
210+
return (uintptr_t)__builtin_frame_address(0);
211+
#elif defined(_MSC_VER)
212+
return (uintptr_t)_AddressOfReturnAddress();
213+
#else
214+
char here;
215+
/* Avoid compiler warning about returning stack address */
216+
return return_pointer_as_int(&here);
217+
#endif
218+
}
219+
201220
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
202221
uintptr_t here_addr = _Py_get_machine_stack_pointer();
203222
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
@@ -239,7 +258,12 @@ PyAPI_FUNC(void) _Py_InitializeRecursionLimits(PyThreadState *tstate);
239258
static inline int _Py_ReachedRecursionLimit(PyThreadState *tstate) {
240259
uintptr_t here_addr = _Py_get_machine_stack_pointer();
241260
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
242-
assert(_tstate->c_stack_hard_limit != 0);
261+
if (here_addr > _tstate->c_stack_soft_limit) {
262+
return 0;
263+
}
264+
if (_tstate->c_stack_hard_limit == 0) {
265+
_Py_InitializeRecursionLimits(tstate);
266+
}
243267
#if _Py_STACK_GROWS_DOWN
244268
return here_addr <= _tstate->c_stack_soft_limit;
245269
#else

‎Include/internal/pycore_pystate.h‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ extern "C" {
1010

1111
#include "pycore_pythonrun.h" // _PyOS_STACK_MARGIN_SHIFT
1212
#include "pycore_typedefs.h" // _PyRuntimeState
13-
#include "pycore_tstate.h"
1413

1514

1615
// Values for PyThreadState.state. A thread must be in the "attached" state
@@ -301,38 +300,6 @@ _Py_AssertHoldsTstateFunc(const char *func)
301300
#define _Py_AssertHoldsTstate()
302301
#endif
303302

304-
#if !_Py__has_builtin(__builtin_frame_address) && !defined(__GNUC__) && !defined(_MSC_VER)
305-
static uintptr_t return_pointer_as_int(char* p) {
306-
return (uintptr_t)p;
307-
}
308-
#endif
309-
310-
static inline uintptr_t
311-
_Py_get_machine_stack_pointer(void) {
312-
#if _Py__has_builtin(__builtin_frame_address) || defined(__GNUC__)
313-
return (uintptr_t)__builtin_frame_address(0);
314-
#elif defined(_MSC_VER)
315-
return (uintptr_t)_AddressOfReturnAddress();
316-
#else
317-
char here;
318-
/* Avoid compiler warning about returning stack address */
319-
return return_pointer_as_int(&here);
320-
#endif
321-
}
322-
323-
static inline intptr_t
324-
_Py_RecursionLimit_GetMargin(PyThreadState *tstate)
325-
{
326-
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
327-
assert(_tstate->c_stack_hard_limit != 0);
328-
intptr_t here_addr = _Py_get_machine_stack_pointer();
329-
#if _Py_STACK_GROWS_DOWN
330-
return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, here_addr - (intptr_t)_tstate->c_stack_soft_limit, _PyOS_STACK_MARGIN_SHIFT);
331-
#else
332-
return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (intptr_t)_tstate->c_stack_soft_limit - here_addr, _PyOS_STACK_MARGIN_SHIFT);
333-
#endif
334-
}
335-
336303
#ifdef __cplusplus
337304
}
338305
#endif

‎Include/pythonrun.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,22 @@ PyAPI_FUNC(void) PyErr_DisplayException(PyObject *);
2121
/* Stuff with no proper home (yet) */
2222
PyAPI_DATA(int) (*PyOS_InputHook)(void);
2323

24+
/* Stack size, in "pointers". This must be large enough, so
25+
* no two calls to check recursion depth are more than this far
26+
* apart. In practice, that means it must be larger than the C
27+
* stack consumption of PyEval_EvalDefault */
28+
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER)
29+
# define PYOS_STACK_MARGIN 4096
30+
#elif defined(Py_DEBUG) && defined(WIN32)
31+
# define PYOS_STACK_MARGIN 4096
32+
#elif defined(__wasi__)
33+
/* Web assembly has two stacks, so this isn't really a size */
34+
# define PYOS_STACK_MARGIN 500
35+
#else
36+
# define PYOS_STACK_MARGIN 2048
37+
#endif
38+
#define PYOS_STACK_MARGIN_BYTES (PYOS_STACK_MARGIN * sizeof(void *))
39+
2440
#if defined(WIN32)
2541
# define USE_STACKCHECK
2642
#endif

‎Modules/_elementtree.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ element_dealloc(PyObject *op)
690690

691691
/* bpo-31095: UnTrack is needed before calling any callbacks */
692692
PyObject_GC_UnTrack(self);
693+
Py_TRASHCAN_BEGIN(self, element_dealloc)
693694

694695
FT_CLEAR_WEAKREFS(op, self->weakreflist);
695696

@@ -699,6 +700,7 @@ element_dealloc(PyObject *op)
699700

700701
tp->tp_free(self);
701702
Py_DECREF(tp);
703+
Py_TRASHCAN_END
702704
}
703705

704706
/* -------------------------------------------------------------------- */

‎Objects/descrobject.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,9 +1311,11 @@ wrapper_dealloc(PyObject *self)
13111311
{
13121312
wrapperobject *wp = (wrapperobject *)self;
13131313
PyObject_GC_UnTrack(wp);
1314+
Py_TRASHCAN_BEGIN(wp, wrapper_dealloc)
13141315
Py_XDECREF(wp->descr);
13151316
Py_XDECREF(wp->self);
13161317
PyObject_GC_Del(wp);
1318+
Py_TRASHCAN_END
13171319
}
13181320

13191321
static PyObject *

‎Objects/dictobject.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3296,6 +3296,7 @@ dict_dealloc(PyObject *self)
32963296

32973297
/* bpo-31095: UnTrack is needed before calling any callbacks */
32983298
PyObject_GC_UnTrack(mp);
3299+
Py_TRASHCAN_BEGIN(mp, dict_dealloc)
32993300
if (values != NULL) {
33003301
if (values->embedded == 0) {
33013302
for (i = 0, n = values->capacity; i < n; i++) {
@@ -3315,6 +3316,7 @@ dict_dealloc(PyObject *self)
33153316
else {
33163317
Py_TYPE(mp)->tp_free((PyObject *)mp);
33173318
}
3319+
Py_TRASHCAN_END
33183320
}
33193321

33203322

‎Objects/exceptions.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,10 @@ BaseException_dealloc(PyObject *op)
150150
// bpo-44348: The trashcan mechanism prevents stack overflow when deleting
151151
// long chains of exceptions. For example, exceptions can be chained
152152
// through the __context__ attributes or the __traceback__ attribute.
153+
Py_TRASHCAN_BEGIN(self, BaseException_dealloc)
153154
(void)BaseException_clear(op);
154155
Py_TYPE(self)->tp_free(self);
156+
Py_TRASHCAN_END
155157
}
156158

157159
static int

‎Objects/frameobject.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,6 +1919,7 @@ frame_dealloc(PyObject *op)
19191919
_PyObject_GC_UNTRACK(f);
19201920
}
19211921

1922+
Py_TRASHCAN_BEGIN(f, frame_dealloc);
19221923
/* GH-106092: If f->f_frame was on the stack and we reached the maximum
19231924
* nesting depth for deallocations, the trashcan may have delayed this
19241925
* deallocation until after f->f_frame is freed. Avoid dereferencing
@@ -1943,6 +1944,7 @@ frame_dealloc(PyObject *op)
19431944
Py_CLEAR(f->f_locals_cache);
19441945
Py_CLEAR(f->f_overwritten_fast_locals);
19451946
PyObject_GC_Del(f);
1947+
Py_TRASHCAN_END;
19461948
}
19471949

19481950
static int

‎Objects/listobject.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,7 @@ list_dealloc(PyObject *self)
551551
PyListObject *op = (PyListObject *)self;
552552
Py_ssize_t i;
553553
PyObject_GC_UnTrack(op);
554+
Py_TRASHCAN_BEGIN(op, list_dealloc)
554555
if (op->ob_item != NULL) {
555556
/* Do it backwards, for Christian Tismer.
556557
There's a simple test case where somehow this reduces
@@ -569,6 +570,7 @@ list_dealloc(PyObject *self)
569570
else {
570571
PyObject_GC_Del(op);
571572
}
573+
Py_TRASHCAN_END
572574
}
573575

574576
static PyObject *

0 commit comments

Comments
 (0)