traverseda
/
cpython
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

bpo-39877: Fix PyEval_RestoreThread() for daemon threads (GH-18811) 

* exit_thread_if_finalizing() does now access directly _PyRuntime
  variable, rather than using tstate->interp->runtime since tstate
  can be a dangling pointer after Py_Finalize() has been called.
* exit_thread_if_finalizing() is now called *before* calling
  take_gil(). _PyRuntime.finalizing is an atomic variable,
  we don't need to hold the GIL to access it.
* Add ensure_tstate_not_null() function to check that tstate is not
  NULL at runtime. Check tstate earlier. take_gil() does not longer
  check if tstate is NULL.

Cleanup:

* PyEval_RestoreThread() no longer saves/restores errno: it's already
  done inside take_gil().
* PyEval_AcquireLock(), PyEval_AcquireThread(),
  PyEval_RestoreThread() and _PyEval_EvalFrameDefault() now check if
  tstate is valid with the new is_tstate_valid() function which uses
  _PyMem_IsPtrFreed().
This commit is contained in:
Victor Stinner 2020-03-08 11:57:45 +01:00 committed by GitHub
parent d5aa2e941c
commit eb4e2ae2b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 74 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
Misc/NEWS.d/next/Core and Builtins/2020-03-06-18-30-00.bpo-39877.bzd1y0.rst Normal file
View File

@ -0,0 +1,5 @@
Fix :c:func:`PyEval_RestoreThread` random crash at exit with daemon threads.
It now accesses the ``_PyRuntime`` variable directly instead of using
``tstate->interp->runtime``, since ``tstate`` can be a dangling pointer after
:c:func:`Py_Finalize` has been called. Moreover, the daemon thread now exits
before trying to take the GIL.

80
Python/ceval.c
View File

@ -188,6 +188,25 @@ static size_t opcache_global_misses = 0;
#include "pythread.h"
#include "ceval_gil.h"
static void
ensure_tstate_not_null(const char *func, PyThreadState *tstate)
{
if (tstate == NULL) {
_Py_FatalErrorFunc(func, "current thread state is NULL");
}
}
#ifndef NDEBUG
static int is_tstate_valid(PyThreadState *tstate)
{
assert(!_PyMem_IsPtrFreed(tstate));
assert(!_PyMem_IsPtrFreed(tstate->interp));
return 1;
}
#endif
int
PyEval_ThreadsInitialized(void)
{
@ -208,6 +227,7 @@ PyEval_InitThreads(void)
PyThread_init_thread();
create_gil(gil);
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
ensure_tstate_not_null(__func__, tstate);
take_gil(ceval, tstate);
struct _pending_calls *pending = &ceval->pending;
@ -235,14 +255,26 @@ _PyEval_FiniThreads(struct _ceval_runtime_state *ceval)
}
}
/* This function is designed to exit daemon threads immediately rather than
taking the GIL if Py_Finalize() has been called.
The caller must *not* hold the GIL, since this function does not release
the GIL before exiting the thread.
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 void
exit_thread_if_finalizing(PyThreadState *tstate)
{
_PyRuntimeState *runtime = tstate->interp->runtime;
/* _Py_Finalizing is protected by the GIL */
/* 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. */
_PyRuntimeState *runtime = &_PyRuntime;
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
if (finalizing != NULL && finalizing != tstate) {
drop_gil(&runtime->ceval, tstate);
PyThread_exit_thread();
}
}
@ -280,13 +312,14 @@ void
PyEval_AcquireLock(void)
{
_PyRuntimeState *runtime = &_PyRuntime;
struct _ceval_runtime_state *ceval = &runtime->ceval;
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
if (tstate == NULL) {
Py_FatalError("current thread state is NULL");
}
take_gil(ceval, tstate);
ensure_tstate_not_null(__func__, tstate);
exit_thread_if_finalizing(tstate);
assert(is_tstate_valid(tstate));
struct _ceval_runtime_state *ceval = &runtime->ceval;
take_gil(ceval, tstate);
}
void
@ -304,15 +337,18 @@ PyEval_ReleaseLock(void)
void
PyEval_AcquireThread(PyThreadState *tstate)
{
assert(tstate != NULL);
ensure_tstate_not_null(__func__, tstate);
exit_thread_if_finalizing(tstate);
assert(is_tstate_valid(tstate));
_PyRuntimeState *runtime = tstate->interp->runtime;
struct _ceval_runtime_state *ceval = &runtime->ceval;
/* Check someone has called PyEval_InitThreads() to create the lock */
assert(gil_created(&ceval->gil));
take_gil(ceval, tstate);
exit_thread_if_finalizing(tstate);
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
Py_FatalError("non-NULL old thread state");
}
@ -344,8 +380,9 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime)
return;
}
recreate_gil(&ceval->gil);
PyThreadState *current_tstate = _PyRuntimeState_GetThreadState(runtime);
take_gil(ceval, current_tstate);
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
ensure_tstate_not_null(__func__, tstate);
take_gil(ceval, tstate);
struct _pending_calls *pending = &ceval->pending;
pending->lock = PyThread_allocate_lock();
@ -354,7 +391,7 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime)
}
/* Destroy all threads except the current one */
_PyThreadState_DeleteExcept(runtime, current_tstate);
_PyThreadState_DeleteExcept(runtime, tstate);
}
/* This function is used to signal that async exceptions are waiting to be
@ -383,16 +420,16 @@ PyEval_SaveThread(void)
void
PyEval_RestoreThread(PyThreadState *tstate)
{
assert(tstate != NULL);
ensure_tstate_not_null(__func__, tstate);
exit_thread_if_finalizing(tstate);
assert(is_tstate_valid(tstate));
_PyRuntimeState *runtime = tstate->interp->runtime;
struct _ceval_runtime_state *ceval = &runtime->ceval;
assert(gil_created(&ceval->gil));
int err = errno;
take_gil(ceval, tstate);
exit_thread_if_finalizing(tstate);
errno = err;
_PyThreadState_Swap(&runtime->gilstate, tstate);
}
@ -750,11 +787,14 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
PyObject **fastlocals, **freevars;
PyObject *retval = NULL; /* Return value */
_PyRuntimeState * const runtime = &_PyRuntime;
PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime);
struct _ceval_runtime_state * const ceval = &runtime->ceval;
_Py_atomic_int * const eval_breaker = &ceval->eval_breaker;
PyCodeObject *co;
PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime);
ensure_tstate_not_null(__func__, tstate);
assert(is_tstate_valid(tstate));
/* when tracing we set things up so that
not (instr_lb <= current_bytecode_offset < instr_ub)
@ -1242,11 +1282,11 @@ main_loop:
/* Other threads may run now */
take_gil(ceval, tstate);
/* Check if we should make a quick exit. */
exit_thread_if_finalizing(tstate);
take_gil(ceval, tstate);
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
Py_FatalError("orphan tstate");
}

11
Python/ceval_gil.h
View File

@ -180,15 +180,17 @@ drop_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
#endif
}
/* Take the GIL.
The function saves errno at entry and restores its value at exit.
tstate must be non-NULL. */
static void
take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
{
if (tstate == NULL) {
Py_FatalError("take_gil: NULL tstate");
}
int err = errno;
struct _gil_runtime_state *gil = &ceval->gil;
int err = errno;
MUTEX_LOCK(gil->mutex);
if (!_Py_atomic_load_relaxed(&gil->locked)) {
@ -240,6 +242,7 @@ _ready:
}
MUTEX_UNLOCK(gil->mutex);
errno = err;
}

4
Python/pylifecycle.c
View File

@ -1364,8 +1364,8 @@ Py_FinalizeEx(void)
int malloc_stats = interp->config.malloc_stats;
#endif
/* Remaining threads (e.g. daemon threads) will automatically exit
after taking the GIL (in PyEval_RestoreThread()). */
/* Remaining daemon threads will automatically exit
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
_PyRuntimeState_SetFinalizing(runtime, tstate);
runtime->initialized = 0;
runtime->core_initialized = 0;