Streamline the trashcan mechanism

Author: markshannon

Include/cpython/object.h

@@ -461,30 +461,29 @@ passed as second argument to Py_TRASHCAN_BEGIN().
 /* Python 3.9 private API, invoked by the macros below. */
 PyAPI_FUNC(int) _PyTrash_begin(PyThreadState *tstate, PyObject *op);
 PyAPI_FUNC(void) _PyTrash_end(PyThreadState *tstate);
+
+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(). */
-PyAPI_FUNC(int) _PyTrash_cond(PyObject *op, destructor dealloc);
 
-#define Py_TRASHCAN_BEGIN_CONDITION(op, cond) \
-    do { \
-        PyThreadState *_tstate = NULL; \
-        /* If "cond" is false, then _tstate remains NULL and the deallocator \
-         * is run normally without involving the trashcan */ \
-        if (cond) { \
-            _tstate = PyThreadState_GetUnchecked(); \
-            if (_PyTrash_begin(_tstate, _PyObject_CAST(op))) { \
-                break; \
-            } \
-        }
-        /* The body of the deallocator is here. */
+#define Py_TRASHCAN_BEGIN(op, dealloc) \
+do { \
+    PyThreadState *tstate = PyThreadState_Get(); \
+    if (tstate->c_recursion_remaining <= 0 && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
+        PyTrash_thread_deposit_object(tstate, (PyObject *)op); \
+        break; \
+    } \
+    tstate->c_recursion_remaining--;
+    /* The body of the deallocator is here. */
 #define Py_TRASHCAN_END \
-        if (_tstate) { \
-            _PyTrash_end(_tstate); \
-        } \
-    } while (0);
+    tstate->c_recursion_remaining++; \
+    if (tstate->delete_later && tstate->c_recursion_remaining > 50) { \
+        PyTrash_thread_destroy_chain(tstate); \
+    } \
+} while (0);
 
-#define Py_TRASHCAN_BEGIN(op, dealloc) \
-    Py_TRASHCAN_BEGIN_CONDITION((op), \
-        _PyTrash_cond(_PyObject_CAST(op), (destructor)(dealloc)))
 
 
 PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj);

Include/cpython/pystate.h

@@ -56,11 +56,6 @@ typedef struct _stack_chunk {
     PyObject * data[1]; /* Variable sized */
 } _PyStackChunk;
 
-struct _py_trashcan {
-    int delete_nesting;
-    PyObject *delete_later;
-};
-
 struct _ts {
     /* See Python/ceval.c for comments explaining most fields */
 
@@ -152,7 +147,7 @@ struct _ts {
      */
     unsigned long native_thread_id;
 
-    struct _py_trashcan trash;
+    PyObject *delete_later;
 
     /* Tagged pointer to top-most critical section, or zero if there is no
      * active critical section. Critical sections are only used in

Objects/object.c

@@ -2692,27 +2692,27 @@ Py_ReprLeave(PyObject *obj)
  * call-stack depth gets large.  op must be a currently untracked gc'ed
  * object, with refcount 0.  Py_DECREF must already have been called on it.
  */
-static void
-_PyTrash_thread_deposit_object(struct _py_trashcan *trash, PyObject *op)
+void
+PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op)
 {
     _PyObject_ASSERT(op, _PyObject_IS_GC(op));
     _PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op));
     _PyObject_ASSERT(op, Py_REFCNT(op) == 0);
 #ifdef Py_GIL_DISABLED
     _PyObject_ASSERT(op, op->ob_tid == 0);
-    op->ob_tid = (uintptr_t)trash->delete_later;
+    op->ob_tid = (uintptr_t)tstate->delete_later;
 #else
-    _PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)trash->delete_later);
+    _PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)tstate->delete_later);
 #endif
-    trash->delete_later = op;
+    tstate->delete_later = op;
 }
 
 /* Deallocate all the objects in the gcstate->trash_delete_later list.
  * Called when the call-stack unwinds again. */
-static void
-_PyTrash_thread_destroy_chain(struct _py_trashcan *trash)
+void
+PyTrash_thread_destroy_chain(PyThreadState *tstate)
 {
-    /* We need to increase trash_delete_nesting here, otherwise,
+    /* We need to increase c_recursion_remaining here, otherwise,
        _PyTrash_thread_destroy_chain will be called recursively
        and then possibly crash.  An example that may crash without
        increase:
@@ -2723,17 +2723,17 @@ _PyTrash_thread_destroy_chain(struct _py_trashcan *trash)
                tups = [(tup,) for tup in tups]
            del tups
     */
-    assert(trash->delete_nesting == 0);
-    ++trash->delete_nesting;
-    while (trash->delete_later) {
-        PyObject *op = trash->delete_later;
+    assert(tstate->c_recursion_remaining > 50);
+    tstate->c_recursion_remaining--;
+    while (tstate->delete_later) {
+        PyObject *op = tstate->delete_later;
         destructor dealloc = Py_TYPE(op)->tp_dealloc;
 
 #ifdef Py_GIL_DISABLED
-        trash->delete_later = (PyObject*) op->ob_tid;
+        tstate->delete_later = (PyObject*) op->ob_tid;
         op->ob_tid = 0;
 #else
-        trash->delete_later = (PyObject*) _PyGCHead_PREV(_Py_AS_GC(op));
+        tstate->delete_later = (PyObject*) _PyGCHead_PREV(_Py_AS_GC(op));
 #endif
 
         /* Call the deallocator directly.  This used to try to
@@ -2744,92 +2744,10 @@ _PyTrash_thread_destroy_chain(struct _py_trashcan *trash)
          */
         _PyObject_ASSERT(op, Py_REFCNT(op) == 0);
         (*dealloc)(op);
-        assert(trash->delete_nesting == 1);
-    }
-    --trash->delete_nesting;
-}
-
-
-static struct _py_trashcan *
-_PyTrash_get_state(PyThreadState *tstate)
-{
-    if (tstate != NULL) {
-        return &tstate->trash;
-    }
-    // The current thread must be finalizing.
-    // Fall back to using thread-local state.
-    // XXX Use thread-local variable syntax?
-    assert(PyThread_tss_is_created(&_PyRuntime.trashTSSkey));
-    struct _py_trashcan *trash =
-        (struct _py_trashcan *)PyThread_tss_get(&_PyRuntime.trashTSSkey);
-    if (trash == NULL) {
-        trash = PyMem_RawMalloc(sizeof(struct _py_trashcan));
-        if (trash == NULL) {
-            Py_FatalError("Out of memory");
-        }
-        PyThread_tss_set(&_PyRuntime.trashTSSkey, (void *)trash);
-    }
-    return trash;
-}
-
-static void
-_PyTrash_clear_state(PyThreadState *tstate)
-{
-    if (tstate != NULL) {
-        assert(tstate->trash.delete_later == NULL);
-        return;
-    }
-    if (PyThread_tss_is_created(&_PyRuntime.trashTSSkey)) {
-        struct _py_trashcan *trash =
-            (struct _py_trashcan *)PyThread_tss_get(&_PyRuntime.trashTSSkey);
-        if (trash != NULL) {
-            PyThread_tss_set(&_PyRuntime.trashTSSkey, (void *)NULL);
-            PyMem_RawFree(trash);
-        }
     }
+    tstate->c_recursion_remaining++;
 }
 
-
-int
-_PyTrash_begin(PyThreadState *tstate, PyObject *op)
-{
-    // XXX Make sure the GIL is held.
-    struct _py_trashcan *trash = _PyTrash_get_state(tstate);
-    if (trash->delete_nesting >= _PyTrash_UNWIND_LEVEL) {
-        /* Store the object (to be deallocated later) and jump past
-         * Py_TRASHCAN_END, skipping the body of the deallocator */
-        _PyTrash_thread_deposit_object(trash, op);
-        return 1;
-    }
-    ++trash->delete_nesting;
-    return 0;
-}
-
-
-void
-_PyTrash_end(PyThreadState *tstate)
-{
-    // XXX Make sure the GIL is held.
-    struct _py_trashcan *trash = _PyTrash_get_state(tstate);
-    --trash->delete_nesting;
-    if (trash->delete_nesting <= 0) {
-        if (trash->delete_later != NULL) {
-            _PyTrash_thread_destroy_chain(trash);
-        }
-        _PyTrash_clear_state(tstate);
-    }
-}
-
-
-/* bpo-40170: It's only be used in Py_TRASHCAN_BEGIN macro to hide
-   implementation details. */
-int
-_PyTrash_cond(PyObject *op, destructor dealloc)
-{
-    return Py_TYPE(op)->tp_dealloc == dealloc;
-}
-
-
 void _Py_NO_RETURN
 _PyObject_AssertFailed(PyObject *obj, const char *expr, const char *msg,
                        const char *file, int line, const char *function)

Python/pystate.c

@@ -1478,6 +1478,8 @@ init_threadstate(_PyThreadStateImpl *_tstate,
     tstate->what_event = -1;
     tstate->previous_executor = NULL;
 
+    tstate->delete_later = NULL;
+
     llist_init(&_tstate->mem_free_queue);
 
     if (interp->stoptheworld.requested || _PyRuntime.stoptheworld.requested) {