diff options
| author | Victor Stinner <vstinner@python.org> | 2023-10-04 11:20:31 (GMT) |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-10-04 11:20:31 (GMT) |
| commit | 4936fa954125864ae3ae5d36863479094837e88a (patch) | |
| tree | 44d8235ceea9c21e887e17cb2e90a20e526020f3 /Modules | |
| parent | 1d87465005e8349323f6dad7e13f48ed5b52f6ad (diff) | |
| download | cpython-4936fa954125864ae3ae5d36863479094837e88a.zip cpython-4936fa954125864ae3ae5d36863479094837e88a.tar.gz cpython-4936fa954125864ae3ae5d36863479094837e88a.tar.bz2 | |
[3.12] gh-108987: Fix _thread.start_new_thread() race condition (#109135) (#110342)
* gh-108987: Fix _thread.start_new_thread() race condition (#109135)
Fix _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.
thread_run() calls PyEval_AcquireThread() which checks if the thread
must exit. The problem was that tstate was dereferenced earlier in
_PyThreadState_Bind() which leads to a crash most of the time.
Move _PyThreadState_CheckConsistency() from thread_run() to
_PyThreadState_Bind().
(cherry picked from commit 517cd82ea7d01b344804413ef05610934a43a241)
* gh-109795: `_thread.start_new_thread`: allocate thread bootstate using raw memory allocator (#109808)
(cherry picked from commit 1b8f2366b38c87b0450d9c15bdfdd4c4a2fc3a01)
---------
Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
Diffstat (limited to 'Modules')
| -rw-r--r-- | Modules/_threadmodule.c | 56 |
1 files changed, 37 insertions, 19 deletions
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 18fd65a..5edb6e9 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1063,22 +1063,22 @@ _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); - PyMem_Free(boot); + if (decref) { + Py_DECREF(boot->func); + Py_DECREF(boot->args); + Py_XDECREF(boot->kwargs); + } + PyMem_RawFree(boot); } @@ -1088,9 +1088,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; + } _PyThreadState_Bind(tstate); PyEval_AcquireThread(tstate); @@ -1109,14 +1124,17 @@ 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 * @@ -1140,7 +1158,6 @@ and False otherwise.\n"); 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, @@ -1179,20 +1196,21 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } - struct bootstate *boot = PyMem_NEW(struct bootstate, 1); + // gh-109795: Use PyMem_RawMalloc() instead of PyMem_Malloc(), + // because it should be possible to call thread_bootstate_free() + // without holding the GIL. + struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate)); if (boot == NULL) { return PyErr_NoMemory(); } - boot->interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_New(boot->interp); + boot->tstate = _PyThreadState_New(interp); if (boot->tstate == NULL) { - PyMem_Free(boot); + PyMem_RawFree(boot); if (!PyErr_Occurred()) { return PyErr_NoMemory(); } return NULL; } - boot->runtime = runtime; boot->func = Py_NewRef(func); boot->args = Py_NewRef(args); boot->kwargs = Py_XNewRef(kwargs); @@ -1201,7 +1219,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); |