diff options
| -rw-r--r-- | Include/internal/pycore_pystate.h | 2 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2023-09-08-12-09-55.gh-issue-108987.x5AIG8.rst | 4 | ||||
| -rw-r--r-- | Modules/_threadmodule.c | 47 | ||||
| -rw-r--r-- | Python/ceval_gil.h | 25 | ||||
| -rw-r--r-- | Python/pystate.c | 26 |
5 files changed, 66 insertions, 38 deletions
diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 8696aaf..7c5aba1 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -65,6 +65,8 @@ _Py_ThreadCanHandlePendingCalls(void) extern int _PyThreadState_CheckConsistency(PyThreadState *tstate); #endif +int _PyThreadState_MustExit(PyThreadState *tstate); + /* Variable and macro for in-line access to current thread and interpreter state */ diff --git a/Misc/NEWS.d/next/Library/2023-09-08-12-09-55.gh-issue-108987.x5AIG8.rst b/Misc/NEWS.d/next/Library/2023-09-08-12-09-55.gh-issue-108987.x5AIG8.rst new file mode 100644 index 0000000..16526ee --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-09-08-12-09-55.gh-issue-108987.x5AIG8.rst @@ -0,0 +1,4 @@ +Fix :func:`_thread.start_new_thread` race condition. If a thread is created +during Python finalization, the newly spawned thread now exits immediately +instead of trying to access freed memory and lead to a crash. Patch by +Victor Stinner. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index b70325a..ac49ee5 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1053,21 +1053,21 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref) /* Module functions */ struct bootstate { - PyInterpreterState *interp; + PyThreadState *tstate; PyObject *func; PyObject *args; PyObject *kwargs; - PyThreadState *tstate; - _PyRuntimeState *runtime; }; static void -thread_bootstate_free(struct bootstate *boot) +thread_bootstate_free(struct bootstate *boot, int decref) { - Py_DECREF(boot->func); - Py_DECREF(boot->args); - Py_XDECREF(boot->kwargs); + if (decref) { + Py_DECREF(boot->func); + Py_DECREF(boot->args); + Py_XDECREF(boot->kwargs); + } PyMem_Free(boot); } @@ -1078,9 +1078,24 @@ thread_run(void *boot_raw) struct bootstate *boot = (struct bootstate *) boot_raw; PyThreadState *tstate = boot->tstate; - // gh-104690: If Python is being finalized and PyInterpreterState_Delete() - // was called, tstate becomes a dangling pointer. - assert(_PyThreadState_CheckConsistency(tstate)); + // gh-108987: If _thread.start_new_thread() is called before or while + // Python is being finalized, thread_run() can called *after*. + // _PyRuntimeState_SetFinalizing() is called. At this point, all Python + // threads must exit, except of the thread calling Py_Finalize() whch holds + // the GIL and must not exit. + // + // At this stage, tstate can be a dangling pointer (point to freed memory), + // it's ok to call _PyThreadState_MustExit() with a dangling pointer. + if (_PyThreadState_MustExit(tstate)) { + // Don't call PyThreadState_Clear() nor _PyThreadState_DeleteCurrent(). + // These functions are called on tstate indirectly by Py_Finalize() + // which calls _PyInterpreterState_Clear(). + // + // Py_DECREF() cannot be called because the GIL is not held: leak + // references on purpose. Python is being finalized anyway. + thread_bootstate_free(boot, 0); + goto exit; + } tstate->thread_id = PyThread_get_thread_ident(); #ifdef PY_HAVE_THREAD_NATIVE_ID @@ -1105,20 +1120,22 @@ thread_run(void *boot_raw) Py_DECREF(res); } - thread_bootstate_free(boot); + thread_bootstate_free(boot, 1); + tstate->interp->threads.count--; PyThreadState_Clear(tstate); _PyThreadState_DeleteCurrent(tstate); +exit: // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with // the glibc, pthread_exit() can abort the whole process if dlopen() fails // to open the libgcc_s.so library (ex: EMFILE error). + return; } static PyObject * thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) { - _PyRuntimeState *runtime = &_PyRuntime; PyObject *func, *args, *kwargs = NULL; if (!PyArg_UnpackTuple(fargs, "start_new_thread", 2, 3, @@ -1151,13 +1168,11 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) if (boot == NULL) { return PyErr_NoMemory(); } - boot->interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_Prealloc(boot->interp); + boot->tstate = _PyThreadState_Prealloc(interp); if (boot->tstate == NULL) { PyMem_Free(boot); return PyErr_NoMemory(); } - boot->runtime = runtime; boot->func = Py_NewRef(func); boot->args = Py_NewRef(args); boot->kwargs = Py_XNewRef(kwargs); @@ -1166,7 +1181,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) if (ident == PYTHREAD_INVALID_THREAD_ID) { PyErr_SetString(ThreadError, "can't start new thread"); PyThreadState_Clear(boot->tstate); - thread_bootstate_free(boot); + thread_bootstate_free(boot, 1); return NULL; } return PyLong_FromUnsignedLong(ident); diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index d20af26..94e2df0 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -185,25 +185,6 @@ drop_gil(struct _ceval_runtime_state *ceval, struct _ceval_state *ceval2, } -/* Check if a Python thread must exit immediately, rather than taking the GIL - if Py_Finalize() has been called. - - When this function is called by a daemon thread after Py_Finalize() has been - called, the GIL does no longer exist. - - tstate must be non-NULL. */ -static inline int -tstate_must_exit(PyThreadState *tstate) -{ - /* bpo-39877: Access _PyRuntime directly rather than using - tstate->interp->runtime to support calls from Python daemon threads. - After Py_Finalize() has been called, tstate can be a dangling pointer: - point to PyThreadState freed memory. */ - PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); - return (finalizing != NULL && finalizing != tstate); -} - - /* Take the GIL. The function saves errno at entry and restores its value at exit. @@ -216,7 +197,7 @@ take_gil(PyThreadState *tstate) assert(tstate != NULL); - if (tstate_must_exit(tstate)) { + if (_PyThreadState_MustExit(tstate)) { /* bpo-39877: If Py_Finalize() has been called and tstate is not the thread which called Py_Finalize(), exit immediately the thread. @@ -255,7 +236,7 @@ take_gil(PyThreadState *tstate) _Py_atomic_load_relaxed(&gil->locked) && gil->switch_number == saved_switchnum) { - if (tstate_must_exit(tstate)) { + if (_PyThreadState_MustExit(tstate)) { MUTEX_UNLOCK(gil->mutex); // gh-96387: If the loop requested a drop request in a previous // iteration, reset the request. Otherwise, drop_gil() can @@ -295,7 +276,7 @@ _ready: MUTEX_UNLOCK(gil->switch_mutex); #endif - if (tstate_must_exit(tstate)) { + if (_PyThreadState_MustExit(tstate)) { /* bpo-36475: If Py_Finalize() has been called and tstate is not the thread which called Py_Finalize(), exit immediately the thread. diff --git a/Python/pystate.c b/Python/pystate.c index ec278ee..db2ce87 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -882,6 +882,10 @@ _PyThreadState_Init(PyThreadState *tstate) void _PyThreadState_SetCurrent(PyThreadState *tstate) { + // gh-104690: If Python is being finalized and PyInterpreterState_Delete() + // was called, tstate becomes a dangling pointer. + assert(_PyThreadState_CheckConsistency(tstate)); + _PyGILState_NoteThreadState(&tstate->interp->runtime->gilstate, tstate); } @@ -2255,6 +2259,28 @@ _PyThreadState_CheckConsistency(PyThreadState *tstate) #endif +// Check if a Python thread must exit immediately, rather than taking the GIL +// if Py_Finalize() has been called. +// +// When this function is called by a daemon thread after Py_Finalize() has been +// called, the GIL does no longer exist. +// +// tstate can be a dangling pointer (point to freed memory): only tstate value +// is used, the pointer is not deferenced. +// +// tstate must be non-NULL. +int +_PyThreadState_MustExit(PyThreadState *tstate) +{ + /* bpo-39877: Access _PyRuntime directly rather than using + tstate->interp->runtime to support calls from Python daemon threads. + After Py_Finalize() has been called, tstate can be a dangling pointer: + point to PyThreadState freed memory. */ + PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); + return (finalizing != NULL && finalizing != tstate); +} + + #ifdef __cplusplus } #endif |