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().
This commit is contained in:
Victor Stinner 2023-09-11 17:27:03 +02:00 committed by GitHub
parent c0f488b88f
commit 517cd82ea7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 69 additions and 41 deletions

View file

@ -329,28 +329,6 @@ drop_gil(struct _ceval_state *ceval, PyThreadState *tstate)
}
/* 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);
if (finalizing == NULL) {
finalizing = _PyInterpreterState_GetFinalizing(tstate->interp);
}
return (finalizing != NULL && finalizing != tstate);
}
/* Take the GIL.
The function saves errno at entry and restores its value at exit.
@ -366,7 +344,7 @@ take_gil(PyThreadState *tstate)
// XXX It may be more correct to check tstate->_status.finalizing.
// XXX assert(!tstate->_status.cleared);
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.
@ -404,7 +382,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
@ -444,7 +422,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.