bpo-39947: Hide implementation detail of trashcan macros (GH-18971)

Py_TRASHCAN_BEGIN_CONDITION and Py_TRASHCAN_END macro no longer
access PyThreadState attributes, but call new private
_PyTrash_begin() and _PyTrash_end() functions which hide
implementation details.

3 files changed, 45 insertions, 15 deletions

diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index 4600f94..45da752 100644
--- a/Include/cpython/object.h
+++ b/Include/cpython/object.h
@@ -385,11 +385,6 @@ PyAPI_DATA(PyTypeObject) _PyNotImplemented_Type;
  */
 PyAPI_DATA(int) _Py_SwappedOp[];
 
-/* This is the old private API, invoked by the macros before 3.2.4.
-   Kept for binary compatibility of extensions using the stable ABI. */
-PyAPI_FUNC(void) _PyTrash_deposit_object(PyObject*);
-PyAPI_FUNC(void) _PyTrash_destroy_chain(void);
-
 PyAPI_FUNC(void)
 _PyDebugAllocatorStats(FILE *out, const char *block_name, int num_blocks,
                        size_t sizeof_block);
@@ -507,10 +502,23 @@ partially-deallocated object. To check this, the tp_dealloc function must be
 passed as second argument to Py_TRASHCAN_BEGIN().
 */
 
-/* The new thread-safe private API, invoked by the macros below. */
+/* This is the old private API, invoked by the macros before 3.2.4.
+   Kept for binary compatibility of extensions using the stable ABI. */
+PyAPI_FUNC(void) _PyTrash_deposit_object(PyObject*);
+PyAPI_FUNC(void) _PyTrash_destroy_chain(void);
+
+/* This is the old private API, invoked by the macros before 3.9.
+   Kept for binary compatibility of extensions using the stable ABI. */
 PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyObject*);
 PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
 
+/* Forward declarations for PyThreadState */
+struct _ts;
+
+/* Python 3.9 private API, invoked by the macros below. */
+PyAPI_FUNC(int) _PyTrash_begin(struct _ts *tstate, PyObject *op);
+PyAPI_FUNC(void) _PyTrash_end(struct _ts *tstate);
+
 #define PyTrash_UNWIND_LEVEL 50
 
 #define Py_TRASHCAN_BEGIN_CONDITION(op, cond) \
@@ -520,24 +528,19 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);
          * is run normally without involving the trashcan */ \
         if (cond) { \
             _tstate = PyThreadState_GET(); \
-            if (_tstate->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(_PyObject_CAST(op)); \
+            if (_PyTrash_begin(_tstate, _PyObject_CAST(op))) { \
                 break; \
             } \
-            ++_tstate->trash_delete_nesting; \
         }
         /* The body of the deallocator is here. */
 #define Py_TRASHCAN_END \
         if (_tstate) { \
-            --_tstate->trash_delete_nesting; \
-            if (_tstate->trash_delete_later && _tstate->trash_delete_nesting <= 0) \
-                _PyTrash_thread_destroy_chain(); \
+            _PyTrash_end(_tstate); \
         } \
     } while (0);
 
-#define Py_TRASHCAN_BEGIN(op, dealloc) Py_TRASHCAN_BEGIN_CONDITION(op, \
+#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
diff --git a/Misc/NEWS.d/next/C API/2020-03-12-23-47-57.bpo-39947.-nCdFV.rst b/Misc/NEWS.d/next/C API/2020-03-12-23-47-57.bpo-39947.-nCdFV.rst
new file mode 100644
index 0000000..559d3dc
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2020-03-12-23-47-57.bpo-39947.-nCdFV.rst
@@ -0,0 +1,3 @@
+Py_TRASHCAN_BEGIN_CONDITION and Py_TRASHCAN_END macro no longer access
+PyThreadState attributes, but call new private _PyTrash_begin() and
+_PyTrash_end() functions which hide implementation details.
diff --git a/Objects/object.c b/Objects/object.c
index 72c4189..e6d0da1 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -2116,6 +2116,30 @@ _PyTrash_thread_destroy_chain(void)
 }
 
 
+int
+_PyTrash_begin(PyThreadState *tstate, PyObject *op)
+{
+    if (tstate->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(op);
+        return 1;
+    }
+    ++tstate->trash_delete_nesting;
+    return 0;
+}
+
+
+void
+_PyTrash_end(PyThreadState *tstate)
+{
+    --tstate->trash_delete_nesting;
+    if (tstate->trash_delete_later && tstate->trash_delete_nesting <= 0) {
+        _PyTrash_thread_destroy_chain();
+    }
+}
+
+
 void _Py_NO_RETURN
 _PyObject_AssertFailed(PyObject *obj, const char *expr, const char *msg,
                        const char *file, int line, const char *function)