pythonGH-124715: Move trashcan mechanism into Py_Dealloc (pythonGH-132280)

Author: markshannon

Include/cpython/object.h

@@ -429,81 +429,14 @@ PyAPI_FUNC(void) _Py_NO_RETURN _PyObject_AssertFailed(
     const char *function);
 
 
-/* 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)/(Py_TRASHCAN_HEADROOM-1) pieces,
-with the call stack never exceeding a depth of Py_TRASHCAN_HEADROOM.
-
-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().
-*/
-
-
 PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op);
 PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(PyThreadState *tstate);
 
-
-/* Python 3.10 private API, invoked by the Py_TRASHCAN_BEGIN(). */
-
-/* To avoid raising recursion errors during dealloc trigger trashcan before we reach
- * recursion limit. To avoid trashing, we don't attempt to empty the trashcan until
- * we have headroom above the trigger limit */
-#define Py_TRASHCAN_HEADROOM 50
-
-/* Helper function for Py_TRASHCAN_BEGIN */
 PyAPI_FUNC(int) _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count);
 
-#define Py_TRASHCAN_BEGIN(op, dealloc) \
-do { \
-    PyThreadState *tstate = PyThreadState_Get(); \
-    if (_Py_ReachedRecursionLimitWithMargin(tstate, 2) && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
-        _PyTrash_thread_deposit_object(tstate, (PyObject *)op); \
-        break; \
-    }
-    /* The body of the deallocator is here. */
-#define Py_TRASHCAN_END \
-    if (tstate->delete_later && !_Py_ReachedRecursionLimitWithMargin(tstate, 4)) { \
-        _PyTrash_thread_destroy_chain(tstate); \
-    } \
-} while (0);
+/* For backwards compatibility with the old trashcan mechanism */
+#define Py_TRASHCAN_BEGIN(op, dealloc)
+#define Py_TRASHCAN_END
 
 
 PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj);

Include/internal/pycore_ceval.h

@@ -196,25 +196,6 @@ extern void _PyEval_DeactivateOpCache(void);
 
 /* --- _Py_EnterRecursiveCall() ----------------------------------------- */
 
-#if !_Py__has_builtin(__builtin_frame_address) && !defined(_MSC_VER)
-static uintptr_t return_pointer_as_int(char* p) {
-    return (uintptr_t)p;
-}
-#endif
-
-static inline uintptr_t
-_Py_get_machine_stack_pointer(void) {
-#if _Py__has_builtin(__builtin_frame_address)
-    return (uintptr_t)__builtin_frame_address(0);
-#elif defined(_MSC_VER)
-    return (uintptr_t)_AddressOfReturnAddress();
-#else
-    char here;
-    /* Avoid compiler warning about returning stack address */
-    return return_pointer_as_int(&here);
-#endif
-}
-
 static inline int _Py_MakeRecCheck(PyThreadState *tstate)  {
     uintptr_t here_addr = _Py_get_machine_stack_pointer();
     _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
@@ -249,12 +230,7 @@ PyAPI_FUNC(void) _Py_InitializeRecursionLimits(PyThreadState *tstate);
 static inline int _Py_ReachedRecursionLimit(PyThreadState *tstate)  {
     uintptr_t here_addr = _Py_get_machine_stack_pointer();
     _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
-    if (here_addr > _tstate->c_stack_soft_limit) {
-        return 0;
-    }
-    if (_tstate->c_stack_hard_limit == 0) {
-        _Py_InitializeRecursionLimits(tstate);
-    }
+    assert(_tstate->c_stack_hard_limit != 0);
     return here_addr <= _tstate->c_stack_soft_limit;
 }
 

Include/internal/pycore_pystate.h

@@ -9,6 +9,7 @@ extern "C" {
 #endif
 
 #include "pycore_typedefs.h"      // _PyRuntimeState
+#include "pycore_tstate.h"
 
 
 // Values for PyThreadState.state. A thread must be in the "attached" state
@@ -299,6 +300,34 @@ _Py_AssertHoldsTstateFunc(const char *func)
 #define _Py_AssertHoldsTstate()
 #endif
 
+#if !_Py__has_builtin(__builtin_frame_address) && !defined(_MSC_VER)
+static uintptr_t return_pointer_as_int(char* p) {
+    return (uintptr_t)p;
+}
+#endif
+
+static inline uintptr_t
+_Py_get_machine_stack_pointer(void) {
+#if _Py__has_builtin(__builtin_frame_address)
+    return (uintptr_t)__builtin_frame_address(0);
+#elif defined(_MSC_VER)
+    return (uintptr_t)_AddressOfReturnAddress();
+#else
+    char here;
+    /* Avoid compiler warning about returning stack address */
+    return return_pointer_as_int(&here);
+#endif
+}
+
+static inline intptr_t
+_Py_RecursionLimit_GetMargin(PyThreadState *tstate)
+{
+    _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
+    assert(_tstate->c_stack_hard_limit != 0);
+    intptr_t here_addr = _Py_get_machine_stack_pointer();
+    return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, here_addr - (intptr_t)_tstate->c_stack_soft_limit, PYOS_STACK_MARGIN_SHIFT);
+}
+
 #ifdef __cplusplus
 }
 #endif

Include/pythonrun.h

@@ -26,17 +26,25 @@ PyAPI_DATA(int) (*PyOS_InputHook)(void);
  * apart. In practice, that means it must be larger than the C
  * stack consumption of PyEval_EvalDefault */
 #if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER)
-#  define PYOS_STACK_MARGIN 4096
+#  define PYOS_LOG2_STACK_MARGIN 12
 #elif defined(Py_DEBUG) && defined(WIN32)
-#  define PYOS_STACK_MARGIN 4096
+#  define PYOS_LOG2_STACK_MARGIN 12
 #elif defined(__wasi__)
    /* Web assembly has two stacks, so this isn't really a size */
-#  define PYOS_STACK_MARGIN 500
+#  define PYOS_LOG2_STACK_MARGIN 9
 #else
-#  define PYOS_STACK_MARGIN 2048
+#  define PYOS_LOG2_STACK_MARGIN 11
 #endif
+#define PYOS_STACK_MARGIN (1 << PYOS_LOG2_STACK_MARGIN)
 #define PYOS_STACK_MARGIN_BYTES (PYOS_STACK_MARGIN * sizeof(void *))
 
+#if SIZEOF_VOID_P == 8
+#define PYOS_STACK_MARGIN_SHIFT (PYOS_LOG2_STACK_MARGIN + 3)
+#else
+#define PYOS_STACK_MARGIN_SHIFT (PYOS_LOG2_STACK_MARGIN + 2)
+#endif
+
+
 #if defined(WIN32)
 #define USE_STACKCHECK
 #endif

Misc/NEWS.d/next/Core_and_Builtins/2025-04-08-17-48-11.gh-issue-124715.xxzQoD.rst

@@ -0,0 +1,3 @@
+Prevents against stack overflows when calling :c:func:`Py_DECREF`. Third-party
+extension objects no longer need to use the "trashcan" mechanism, as
+protection is now built into the :c:func:`Py_DECREF` macro.

Modules/_elementtree.c

@@ -689,7 +689,6 @@ element_dealloc(PyObject *op)
 
     /* 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(op);
@@ -700,7 +699,6 @@ element_dealloc(PyObject *op)
 
     tp->tp_free(self);
     Py_DECREF(tp);
-    Py_TRASHCAN_END
 }
 
 /* -------------------------------------------------------------------- */

Objects/descrobject.c

@@ -1311,11 +1311,9 @@ wrapper_dealloc(PyObject *self)
 {
     wrapperobject *wp = (wrapperobject *)self;
     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 *

Objects/dictobject.c

@@ -3285,7 +3285,6 @@ dict_dealloc(PyObject *self)
 
     /* bpo-31095: UnTrack is needed before calling any callbacks */
     PyObject_GC_UnTrack(mp);
-    Py_TRASHCAN_BEGIN(mp, dict_dealloc)
     if (values != NULL) {
         if (values->embedded == 0) {
             for (i = 0, n = values->capacity; i < n; i++) {
@@ -3305,7 +3304,6 @@ dict_dealloc(PyObject *self)
     else {
         Py_TYPE(mp)->tp_free((PyObject *)mp);
     }
-    Py_TRASHCAN_END
 }
 
 

Objects/exceptions.c

@@ -150,10 +150,8 @@ BaseException_dealloc(PyObject *op)
     // bpo-44348: The trashcan mechanism prevents stack overflow when deleting
     // long chains of exceptions. For example, exceptions can be chained
     // through the __context__ attributes or the __traceback__ attribute.
-    Py_TRASHCAN_BEGIN(self, BaseException_dealloc)
     (void)BaseException_clear(op);
     Py_TYPE(self)->tp_free(self);
-    Py_TRASHCAN_END
 }
 
 static int

Objects/frameobject.c

@@ -1917,7 +1917,6 @@ frame_dealloc(PyObject *op)
         _PyObject_GC_UNTRACK(f);
     }
 
-    Py_TRASHCAN_BEGIN(f, frame_dealloc);
     /* GH-106092: If f->f_frame was on the stack and we reached the maximum
      * nesting depth for deallocations, the trashcan may have delayed this
      * deallocation until after f->f_frame is freed. Avoid dereferencing
@@ -1942,7 +1941,6 @@ frame_dealloc(PyObject *op)
     Py_CLEAR(f->f_locals_cache);
     Py_CLEAR(f->f_overwritten_fast_locals);
     PyObject_GC_Del(f);
-    Py_TRASHCAN_END;
 }
 
 static int