bpo-35983: improve and test old trashcan macros

Author: jdemeyer

Include/object.h

@@ -674,6 +674,10 @@ CAUTION:  Never return from the middle of the body!  If the body needs to
 call, and goto it.  Else the call-depth counter (see below) will stay
 above 0 forever, and the trashcan will never get emptied.
 
+CAUTION:  Any object using the trashcan must be an untracked GC object.
+This is because the implementation of the trashcan reuses the GC data
+structure.
+
 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
@@ -727,9 +731,12 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
 #define Py_TRASHCAN_BEGIN(op, dealloc) Py_TRASHCAN_BEGIN_CONDITION(op, \
         Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))
 
-/* For backwards compatibility, these macros enable the trashcan
- * unconditionally */
-#define Py_TRASHCAN_SAFE_BEGIN(op) Py_TRASHCAN_BEGIN_CONDITION(op, 1)
+/* Old trashcan macros which don't take a "dealloc" argument. To be safe, we
+ * enable the trashcan only for classes inheriting directly from "object",
+ * where the problem of a subclass also using the trashcan does not appear.
+ * This is probably the most common case anyway. */
+#define Py_TRASHCAN_SAFE_BEGIN(op) Py_TRASHCAN_BEGIN_CONDITION(op, \
+        Py_TYPE(op)->tp_base == &PyBaseObject_Type)
 #define Py_TRASHCAN_SAFE_END(op) Py_TRASHCAN_END
 
 

Lib/test/test_capi.py

@@ -341,6 +341,15 @@ def test_trashcan_subclass(self):
         for i in range(1000):
             L = MyList((L,))
 
+    def test_trashcan_compat(self):
+        # bpo-35983: Check that the trashcan works with the
+        # backwards-compatibility macros
+        # Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END
+        from _testcapi import SingleContainer
+        L = None
+        for i in range(2**20):
+            L = SingleContainer(L)
+
     def test_trashcan_python_class(self):
         # Check that the trashcan mechanism works properly for a Python
         # subclass of a class using the trashcan (list in this test)

Lib/test/test_descr.py

@@ -4307,7 +4307,7 @@ class Oops(object):
         o.whatever = Provoker(o)
         del o
 
-    def test_wrapper_segfault(self):
+    def test_wrapper_trashcan(self):
         # SF 927248: deeply nested wrappers could cause stack overflow
         f = lambda:None
         for i in range(1000000):

Modules/_testcapimodule.c

@@ -5399,6 +5399,85 @@ static PyTypeObject MyList_Type = {
     MyList_new,                                 /* tp_new */
 };
 
+/* Test bpo-35983: trashcan backwards compatibility macros */
+
+typedef struct {
+    PyObject_HEAD
+    PyObject *item;
+} SingleContainerObject;
+
+static PyObject *
+SingleContainer_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
+{
+    PyObject *obj;
+    if (!_PyArg_NoKeywords("SingleContainer", kwargs)) {
+        return NULL;
+    }
+    if (!PyArg_UnpackTuple(args, "SingleContainer", 1, 1, &obj)) {
+        return NULL;
+    }
+    SingleContainerObject *op = PyObject_GC_New(SingleContainerObject, type);
+    if (op) {
+        Py_INCREF(obj);
+        op->item = obj;
+    }
+    return (PyObject *)op;
+}
+
+void
+SingleContainer_dealloc(SingleContainerObject* op)
+{
+    /* Intentionally use the old
+     * Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END macros */
+    PyObject_GC_UnTrack(op);
+    Py_TRASHCAN_SAFE_BEGIN(op);
+    Py_DECREF(op->item);
+    PyObject_GC_Del(op);
+    Py_TRASHCAN_SAFE_END(op);
+}
+
+static PyTypeObject SingleContainer_Type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "SingleContainer",
+    sizeof(SingleContainerObject),
+    0,
+    (destructor)SingleContainer_dealloc,        /* tp_dealloc */
+    0,                                          /* tp_print */
+    0,                                          /* tp_getattr */
+    0,                                          /* tp_setattr */
+    0,                                          /* tp_reserved */
+    0,                                          /* tp_repr */
+    0,                                          /* tp_as_number */
+    0,                                          /* tp_as_sequence */
+    0,                                          /* tp_as_mapping */
+    0,                                          /* tp_hash */
+    0,                                          /* tp_call */
+    0,                                          /* tp_str */
+    0,                                          /* tp_getattro */
+    0,                                          /* tp_setattro */
+    0,                                          /* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
+                         Py_TPFLAGS_HAVE_GC,    /* tp_flags */
+    0,                                          /* tp_doc */
+    0,                                          /* tp_traverse */
+    0,                                          /* tp_clear */
+    0,                                          /* tp_richcompare */
+    0,                                          /* tp_weaklistoffset */
+    0,                                          /* tp_iter */
+    0,                                          /* tp_iternext */
+    0,                                          /* tp_methods */
+    0,                                          /* tp_members */
+    0,                                          /* tp_getset */
+    0,                                          /* tp_base */
+    0,                                          /* tp_dict */
+    0,                                          /* tp_descr_get */
+    0,                                          /* tp_descr_set */
+    0,                                          /* tp_dictoffset */
+    0,                                          /* tp_init */
+    0,                                          /* tp_alloc */
+    SingleContainer_new                         /* tp_new */
+};
+
 
 /* Test PEP 560 */
 
@@ -5519,6 +5598,11 @@ PyInit__testcapi(void)
     Py_INCREF(&MyList_Type);
     PyModule_AddObject(m, "MyList", (PyObject *)&MyList_Type);
 
+    if (PyType_Ready(&SingleContainer_Type) < 0)
+        return NULL;
+    Py_INCREF(&SingleContainer_Type);
+    PyModule_AddObject(m, "SingleContainer", (PyObject *)&SingleContainer_Type);
+
     if (PyType_Ready(&GenericAlias_Type) < 0)
         return NULL;
     Py_INCREF(&GenericAlias_Type);