From 26baa747c2ebc2beeff769bb07b5fb5a51ad5f4b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 15 May 2023 13:59:26 -0600 Subject: [PATCH] gh-104341: Adjust tstate_must_exit() to Respect Interpreter Finalization (gh-104437) With the move to a per-interpreter GIL, this check slipped through the cracks. --- Include/cpython/pylifecycle.h | 1 + Include/internal/pycore_interp.h | 18 ++++++++++++++++++ Modules/_asynciomodule.c | 2 +- Modules/_io/bufferedio.c | 3 ++- Modules/_sqlite/connection.c | 2 +- Modules/_winapi.c | 2 +- Python/_warnings.c | 2 +- Python/ceval_gil.c | 3 +++ Python/pylifecycle.c | 17 +++++++++++++++++ Python/pystate.c | 12 +++++++----- Python/sysmodule.c | 4 ++++ 11 files changed, 56 insertions(+), 10 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index 08569ee683ce0d..314a5cc5b942ac 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -52,6 +52,7 @@ PyAPI_FUNC(const char *) _Py_gitidentifier(void); PyAPI_FUNC(const char *) _Py_gitversion(void); PyAPI_FUNC(int) _Py_IsFinalizing(void); +PyAPI_FUNC(int) _Py_IsInterpreterFinalizing(PyInterpreterState *interp); /* Random */ PyAPI_FUNC(int) _PyOS_URandom(void *buffer, Py_ssize_t size); diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 527b2121148f4c..edc076fc04f6c3 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -83,6 +83,13 @@ struct _is { int _initialized; int finalizing; + /* Set by Py_EndInterpreter(). + + Use _PyInterpreterState_GetFinalizing() + and _PyInterpreterState_SetFinalizing() + to access it, don't access it directly. */ + _Py_atomic_address _finalizing; + struct _obmalloc_state obmalloc; struct _ceval_state ceval; @@ -191,6 +198,17 @@ struct _is { extern void _PyInterpreterState_Clear(PyThreadState *tstate); +static inline PyThreadState* +_PyInterpreterState_GetFinalizing(PyInterpreterState *interp) { + return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing); +} + +static inline void +_PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) { + _Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate); +} + + /* cross-interpreter data registry */ /* For now we use a global registry of shareable classes. An diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 3830245abe87b3..7e33558dba3e32 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -526,7 +526,7 @@ future_init(FutureObj *fut, PyObject *loop) if (is_true < 0) { return -1; } - if (is_true && !_Py_IsFinalizing()) { + if (is_true && !_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) { /* Only try to capture the traceback if the interpreter is not being finalized. The original motivation to add a `_Py_IsFinalizing()` call was to prevent SIGSEGV when a Future is created in a __del__ diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 6f291c34496064..7a0c516411c73b 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -293,7 +293,8 @@ _enter_buffered_busy(buffered *self) "reentrant call inside %R", self); return 0; } - relax_locking = _Py_IsFinalizing(); + PyInterpreterState *interp = PyInterpreterState_Get(); + relax_locking = _Py_IsInterpreterFinalizing(interp); Py_BEGIN_ALLOW_THREADS if (!relax_locking) st = PyThread_acquire_lock(self->lock, 1); diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 7bbb462ed54dfa..5c57a4101c4a69 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -419,7 +419,7 @@ connection_close(pysqlite_Connection *self) { /* If close is implicitly called as a result of interpreter * tear-down, we must not call back into Python. */ - if (_Py_IsFinalizing()) { + if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) { remove_callbacks(self->db); } (void)connection_exec_stmt(self, "ROLLBACK"); diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 473bcb4736e925..1e02dbc1a4bfd1 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -133,7 +133,7 @@ overlapped_dealloc(OverlappedObject *self) { /* The operation is no longer pending -- nothing to do. */ } - else if (_Py_IsFinalizing()) + else if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) { /* The operation is still pending -- give a warning. This will probably only happen on Windows XP. */ diff --git a/Python/_warnings.c b/Python/_warnings.c index 5644db9a3770cb..dec658680241ed 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -198,7 +198,7 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) PyObject *warnings_module, *obj; /* don't try to import after the start of the Python finallization */ - if (try_import && !_Py_IsFinalizing()) { + if (try_import && !_Py_IsInterpreterFinalizing(interp)) { warnings_module = PyImport_Import(&_Py_ID(warnings)); if (warnings_module == NULL) { /* Fallback to the C implementation if we cannot get diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 42e1436bc9130d..b9bdb74fcedf32 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -332,6 +332,9 @@ tstate_must_exit(PyThreadState *tstate) After Py_Finalize() has been called, tstate can be a dangling pointer: point to PyThreadState freed memory. */ PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); + if (finalizing == NULL) { + finalizing = _PyInterpreterState_GetFinalizing(tstate->interp); + } return (finalizing != NULL && finalizing != tstate); } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index c5dc0f44a38068..cb87f2c0860110 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1788,6 +1788,7 @@ Py_FinalizeEx(void) /* Remaining daemon threads will automatically exit when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ + _PyInterpreterState_SetFinalizing(tstate->interp, tstate); _PyRuntimeState_SetFinalizing(runtime, tstate); runtime->initialized = 0; runtime->core_initialized = 0; @@ -2142,6 +2143,10 @@ Py_EndInterpreter(PyThreadState *tstate) Py_FatalError("not the last thread"); } + /* Remaining daemon threads will automatically exit + when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ + _PyInterpreterState_SetFinalizing(interp, tstate); + // XXX Call something like _PyImport_Disable() here? _PyImport_FiniExternal(tstate->interp); @@ -2152,6 +2157,18 @@ Py_EndInterpreter(PyThreadState *tstate) finalize_interp_delete(tstate->interp); } +int +_Py_IsInterpreterFinalizing(PyInterpreterState *interp) +{ + /* We check the runtime first since, in a daemon thread, + interp might be dangling pointer. */ + PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); + if (finalizing == NULL) { + finalizing = _PyInterpreterState_GetFinalizing(interp); + } + return finalizing != NULL; +} + /* Add the __main__ module */ static PyStatus diff --git a/Python/pystate.c b/Python/pystate.c index 26debf1f88b94a..25e655a2027918 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1436,11 +1436,13 @@ PyThreadState_Clear(PyThreadState *tstate) if (verbose && tstate->cframe->current_frame != NULL) { /* bpo-20526: After the main thread calls - _PyRuntimeState_SetFinalizing() in Py_FinalizeEx(), threads must - exit when trying to take the GIL. If a thread exit in the middle of - _PyEval_EvalFrameDefault(), tstate->frame is not reset to its - previous value. It is more likely with daemon threads, but it can - happen with regular threads if threading._shutdown() fails + _PyInterpreterState_SetFinalizing() in Py_FinalizeEx() + (or in Py_EndInterpreter() for subinterpreters), + threads must exit when trying to take the GIL. + If a thread exit in the middle of _PyEval_EvalFrameDefault(), + tstate->frame is not reset to its previous value. + It is more likely with daemon threads, but it can happen + with regular threads if threading._shutdown() fails (ex: interrupted by CTRL+C). */ fprintf(stderr, "PyThreadState_Clear: warning: thread still has a frame\n"); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 894a3e8a98fd8a..c116c40538a211 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -332,6 +332,7 @@ _PySys_ClearAuditHooks(PyThreadState *ts) } _PyRuntimeState *runtime = ts->interp->runtime; + /* The hooks are global so we have to check for runtime finalization. */ PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); assert(finalizing == ts); if (finalizing != ts) { @@ -2039,6 +2040,9 @@ sys__clear_type_cache_impl(PyObject *module) Py_RETURN_NONE; } +/* Note that, for now, we do not have a per-interpreter equivalent + for sys.is_finalizing(). */ + /*[clinic input] sys.is_finalizing