gh-116522: Refactor _PyThreadState_DeleteExcept (#117131)

Split `_PyThreadState_DeleteExcept` into two functions:

- `_PyThreadState_RemoveExcept` removes all thread states other than one
  passed as an argument. It returns the removed thread states as a
  linked list.

- `_PyThreadState_DeleteList` deletes those dead thread states. It may
  call destructors, so we want to "start the world" before calling
  `_PyThreadState_DeleteList` to avoid potential deadlocks.
This commit is contained in:
Sam Gross 2024-03-21 14:21:02 -04:00 committed by GitHub
parent 50369e6c34
commit 1f72fb5447
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 41 additions and 23 deletions

View file

@ -218,7 +218,8 @@ extern PyThreadState * _PyThreadState_New(
PyInterpreterState *interp, PyInterpreterState *interp,
int whence); int whence);
extern void _PyThreadState_Bind(PyThreadState *tstate); extern void _PyThreadState_Bind(PyThreadState *tstate);
extern void _PyThreadState_DeleteExcept(PyThreadState *tstate); extern PyThreadState * _PyThreadState_RemoveExcept(PyThreadState *tstate);
extern void _PyThreadState_DeleteList(PyThreadState *list);
extern void _PyThreadState_ClearMimallocHeaps(PyThreadState *tstate); extern void _PyThreadState_ClearMimallocHeaps(PyThreadState *tstate);
// Export for '_testinternalcapi' shared extension // Export for '_testinternalcapi' shared extension

View file

@ -664,6 +664,14 @@ PyOS_AfterFork_Child(void)
goto fatal_error; goto fatal_error;
} }
// Remove the dead thread states. We "start the world" once we are the only
// thread state left to undo the stop the world call in `PyOS_BeforeFork`.
// That needs to happen before `_PyThreadState_DeleteList`, because that
// may call destructors.
PyThreadState *list = _PyThreadState_RemoveExcept(tstate);
_PyEval_StartTheWorldAll(&_PyRuntime);
_PyThreadState_DeleteList(list);
status = _PyImport_ReInitLock(tstate->interp); status = _PyImport_ReInitLock(tstate->interp);
if (_PyStatus_EXCEPTION(status)) { if (_PyStatus_EXCEPTION(status)) {
goto fatal_error; goto fatal_error;

View file

@ -579,9 +579,8 @@ PyEval_ReleaseThread(PyThreadState *tstate)
} }
#ifdef HAVE_FORK #ifdef HAVE_FORK
/* This function is called from PyOS_AfterFork_Child to destroy all threads /* This function is called from PyOS_AfterFork_Child to re-initialize the
which are not running in the child process, and clear internal locks GIL and pending calls lock. */
which might be held by those threads. */
PyStatus PyStatus
_PyEval_ReInitThreads(PyThreadState *tstate) _PyEval_ReInitThreads(PyThreadState *tstate)
{ {
@ -598,8 +597,6 @@ _PyEval_ReInitThreads(PyThreadState *tstate)
struct _pending_calls *pending = &tstate->interp->ceval.pending; struct _pending_calls *pending = &tstate->interp->ceval.pending;
_PyMutex_at_fork_reinit(&pending->mutex); _PyMutex_at_fork_reinit(&pending->mutex);
/* Destroy all threads except the current one */
_PyThreadState_DeleteExcept(tstate);
return _PyStatus_OK(); return _PyStatus_OK();
} }
#endif #endif

View file

@ -1934,8 +1934,11 @@ Py_FinalizeEx(void)
will be called in the current Python thread. Since will be called in the current Python thread. Since
_PyRuntimeState_SetFinalizing() has been called, no other Python thread _PyRuntimeState_SetFinalizing() has been called, no other Python thread
can take the GIL at this point: if they try, they will exit can take the GIL at this point: if they try, they will exit
immediately. */ immediately. We start the world once we are the only thread state left,
_PyThreadState_DeleteExcept(tstate); before we call destructors. */
PyThreadState *list = _PyThreadState_RemoveExcept(tstate);
_PyEval_StartTheWorldAll(runtime);
_PyThreadState_DeleteList(list);
/* At this point no Python code should be running at all. /* At this point no Python code should be running at all.
The only thread state left should be the main thread of the main The only thread state left should be the main thread of the main

View file

@ -1763,15 +1763,17 @@ PyThreadState_DeleteCurrent(void)
} }
/* // Unlinks and removes all thread states from `tstate->interp`, with the
* Delete all thread states except the one passed as argument. // exception of the one passed as an argument. However, it does not delete
* Note that, if there is a current thread state, it *must* be the one // these thread states. Instead, it returns the removed thread states as a
* passed as argument. Also, this won't touch any other interpreters // linked list.
* than the current one, since we don't know which thread state should //
* be kept in those other interpreters. // Note that if there is a current thread state, it *must* be the one
*/ // passed as argument. Also, this won't touch any interpreters other
void // than the current one, since we don't know which thread state should
_PyThreadState_DeleteExcept(PyThreadState *tstate) // be kept in those other interpreters.
PyThreadState *
_PyThreadState_RemoveExcept(PyThreadState *tstate)
{ {
assert(tstate != NULL); assert(tstate != NULL);
PyInterpreterState *interp = tstate->interp; PyInterpreterState *interp = tstate->interp;
@ -1783,8 +1785,7 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
HEAD_LOCK(runtime); HEAD_LOCK(runtime);
/* Remove all thread states, except tstate, from the linked list of /* Remove all thread states, except tstate, from the linked list of
thread states. This will allow calling PyThreadState_Clear() thread states. */
without holding the lock. */
PyThreadState *list = interp->threads.head; PyThreadState *list = interp->threads.head;
if (list == tstate) { if (list == tstate) {
list = tstate->next; list = tstate->next;
@ -1799,11 +1800,19 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
interp->threads.head = tstate; interp->threads.head = tstate;
HEAD_UNLOCK(runtime); HEAD_UNLOCK(runtime);
_PyEval_StartTheWorldAll(runtime); return list;
}
// Deletes the thread states in the linked list `list`.
//
// This is intended to be used in conjunction with _PyThreadState_RemoveExcept.
void
_PyThreadState_DeleteList(PyThreadState *list)
{
// The world can't be stopped because we PyThreadState_Clear() can
// call destructors.
assert(!_PyRuntime.stoptheworld.world_stopped);
/* Clear and deallocate all stale thread states. Even if this
executes Python code, we should be safe since it executes
in the current thread, not one of the stale threads. */
PyThreadState *p, *next; PyThreadState *p, *next;
for (p = list; p; p = next) { for (p = list; p; p = next) {
next = p->next; next = p->next;