From 7b20a0f55a16b3e2d274cc478e4d04bd8a836a9f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 23 Jan 2023 08:30:20 -0700 Subject: gh-59956: Allow the "Trashcan" Mechanism to Work Without a Thread State (gh-101209) We've factored out a struct from the two PyThreadState fields. This accomplishes two things: * make it clear that the trashcan-related code doesn't need any other parts of PyThreadState * allows us to use the trashcan mechanism even when there isn't a "current" thread state We still expect the caller to hold the GIL. https://github.com/python/cpython/issues/59956 --- Include/cpython/object.h | 2 +- Include/cpython/pystate.h | 8 +++- Include/internal/pycore_runtime.h | 3 ++ Objects/object.c | 87 +++++++++++++++++++++++++++++---------- Python/pystate.c | 16 +++++++ 5 files changed, 92 insertions(+), 24 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 4263370..3f26f24 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -507,7 +507,7 @@ PyAPI_FUNC(int) _PyTrash_cond(PyObject *op, destructor dealloc); /* If "cond" is false, then _tstate remains NULL and the deallocator \ * is run normally without involving the trashcan */ \ if (cond) { \ - _tstate = PyThreadState_Get(); \ + _tstate = _PyThreadState_UncheckedGet(); \ if (_PyTrash_begin(_tstate, _PyObject_CAST(op))) { \ break; \ } \ diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 81944a5..3c1e70d 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -107,6 +107,11 @@ 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 */ @@ -160,8 +165,7 @@ struct _ts { */ unsigned long native_thread_id; - int trash_delete_nesting; - PyObject *trash_delete_later; + struct _py_trashcan trash; /* Called when a thread state is deleted normally, but not when it * is destroyed after fork(). diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 41e1b2c..9ef2707 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -126,6 +126,9 @@ typedef struct pyruntimestate { /* Used for the thread state bound to the current thread. */ Py_tss_t autoTSSkey; + /* Used instead of PyThreadState.trash when there is not current tstate. */ + Py_tss_t trashTSSkey; + PyWideStringList orig_argv; struct _parser_runtime_state parser; diff --git a/Objects/object.c b/Objects/object.c index fae508c..e940222 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2225,22 +2225,20 @@ finally: * object, with refcount 0. Py_DECREF must already have been called on it. */ static void -_PyTrash_thread_deposit_object(PyObject *op) +_PyTrash_thread_deposit_object(struct _py_trashcan *trash, PyObject *op) { - PyThreadState *tstate = _PyThreadState_GET(); _PyObject_ASSERT(op, _PyObject_IS_GC(op)); _PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op)); _PyObject_ASSERT(op, Py_REFCNT(op) == 0); - _PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)tstate->trash_delete_later); - tstate->trash_delete_later = op; + _PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)trash->delete_later); + trash->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(void) +_PyTrash_thread_destroy_chain(struct _py_trashcan *trash) { - PyThreadState *tstate = _PyThreadState_GET(); /* We need to increase trash_delete_nesting here, otherwise, _PyTrash_thread_destroy_chain will be called recursively and then possibly crash. An example that may crash without @@ -2252,13 +2250,13 @@ _PyTrash_thread_destroy_chain(void) tups = [(tup,) for tup in tups] del tups */ - assert(tstate->trash_delete_nesting == 0); - ++tstate->trash_delete_nesting; - while (tstate->trash_delete_later) { - PyObject *op = tstate->trash_delete_later; + assert(trash->delete_nesting == 0); + ++trash->delete_nesting; + while (trash->delete_later) { + PyObject *op = trash->delete_later; destructor dealloc = Py_TYPE(op)->tp_dealloc; - tstate->trash_delete_later = + trash->delete_later = (PyObject*) _PyGCHead_PREV(_Py_AS_GC(op)); /* Call the deallocator directly. This used to try to @@ -2269,22 +2267,64 @@ _PyTrash_thread_destroy_chain(void) */ _PyObject_ASSERT(op, Py_REFCNT(op) == 0); (*dealloc)(op); - assert(tstate->trash_delete_nesting == 1); + 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->trash_delete_nesting; } int _PyTrash_begin(PyThreadState *tstate, PyObject *op) { - if (tstate->trash_delete_nesting >= _PyTrash_UNWIND_LEVEL) { + // 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(op); + _PyTrash_thread_deposit_object(trash, op); return 1; } - ++tstate->trash_delete_nesting; + ++trash->delete_nesting; return 0; } @@ -2292,9 +2332,14 @@ _PyTrash_begin(PyThreadState *tstate, PyObject *op) void _PyTrash_end(PyThreadState *tstate) { - --tstate->trash_delete_nesting; - if (tstate->trash_delete_later && tstate->trash_delete_nesting <= 0) { - _PyTrash_thread_destroy_chain(); + // 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); } } @@ -2371,7 +2416,7 @@ _Py_Dealloc(PyObject *op) destructor dealloc = type->tp_dealloc; #ifdef Py_DEBUG PyThreadState *tstate = _PyThreadState_GET(); - PyObject *old_exc_type = tstate->curexc_type; + PyObject *old_exc_type = tstate != NULL ? tstate->curexc_type : NULL; // Keep the old exception type alive to prevent undefined behavior // on (tstate->curexc_type != old_exc_type) below Py_XINCREF(old_exc_type); @@ -2387,7 +2432,7 @@ _Py_Dealloc(PyObject *op) #ifdef Py_DEBUG // gh-89373: The tp_dealloc function must leave the current exception // unchanged. - if (tstate->curexc_type != old_exc_type) { + if (tstate != NULL && tstate->curexc_type != old_exc_type) { const char *err; if (old_exc_type == NULL) { err = "Deallocator of type '%s' raised an exception"; diff --git a/Python/pystate.c b/Python/pystate.c index 5c1636a..d31c1f1 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -400,6 +400,11 @@ _PyRuntimeState_Init(_PyRuntimeState *runtime) return status; } + if (PyThread_tss_create(&runtime->trashTSSkey) != 0) { + _PyRuntimeState_Fini(runtime); + return _PyStatus_NO_MEMORY(); + } + init_runtime(runtime, open_code_hook, open_code_userdata, audit_hook_head, unicode_next_index, lock1, lock2, lock3, lock4); @@ -413,6 +418,10 @@ _PyRuntimeState_Fini(_PyRuntimeState *runtime) current_tss_fini(runtime); } + if (PyThread_tss_is_created(&runtime->trashTSSkey)) { + PyThread_tss_delete(&runtime->trashTSSkey); + } + /* Force the allocator used by _PyRuntimeState_Init(). */ PyMemAllocatorEx old_alloc; _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); @@ -471,6 +480,13 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) return status; } + if (PyThread_tss_is_created(&runtime->trashTSSkey)) { + PyThread_tss_delete(&runtime->trashTSSkey); + } + if (PyThread_tss_create(&runtime->trashTSSkey) != 0) { + return _PyStatus_NO_MEMORY(); + } + return _PyStatus_OK(); } #endif -- cgit v0.12