Skip to content

Commit eb4e2ae

Browse files
authored
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().
1 parent d5aa2e9 commit eb4e2ae

File tree

4 files changed

+74
-26
lines changed

4 files changed

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

Python/ceval.c

+60-20
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,25 @@ static size_t opcache_global_misses = 0;
188188
#include "pythread.h"
189189
#include "ceval_gil.h"
190190

191+
static void
192+
ensure_tstate_not_null(const char *func, PyThreadState *tstate)
193+
{
194+
if (tstate == NULL) {
195+
_Py_FatalErrorFunc(func, "current thread state is NULL");
196+
}
197+
}
198+
199+
200+
#ifndef NDEBUG
201+
static int is_tstate_valid(PyThreadState *tstate)
202+
{
203+
assert(!_PyMem_IsPtrFreed(tstate));
204+
assert(!_PyMem_IsPtrFreed(tstate->interp));
205+
return 1;
206+
}
207+
#endif
208+
209+
191210
int
192211
PyEval_ThreadsInitialized(void)
193212
{
@@ -208,6 +227,7 @@ PyEval_InitThreads(void)
208227
PyThread_init_thread();
209228
create_gil(gil);
210229
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
230+
ensure_tstate_not_null(__func__, tstate);
211231
take_gil(ceval, tstate);
212232

213233
struct _pending_calls *pending = &ceval->pending;
@@ -235,14 +255,26 @@ _PyEval_FiniThreads(struct _ceval_runtime_state *ceval)
235255
}
236256
}
237257

258+
/* This function is designed to exit daemon threads immediately rather than
259+
taking the GIL if Py_Finalize() has been called.
260+
261+
The caller must *not* hold the GIL, since this function does not release
262+
the GIL before exiting the thread.
263+
264+
When this function is called by a daemon thread after Py_Finalize() has been
265+
called, the GIL does no longer exist.
266+
267+
tstate must be non-NULL. */
238268
static inline void
239269
exit_thread_if_finalizing(PyThreadState *tstate)
240270
{
241-
_PyRuntimeState *runtime = tstate->interp->runtime;
242-
/* _Py_Finalizing is protected by the GIL */
271+
/* bpo-39877: Access _PyRuntime directly rather than using
272+
tstate->interp->runtime to support calls from Python daemon threads.
273+
After Py_Finalize() has been called, tstate can be a dangling pointer:
274+
point to PyThreadState freed memory. */
275+
_PyRuntimeState *runtime = &_PyRuntime;
243276
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
244277
if (finalizing != NULL && finalizing != tstate) {
245-
drop_gil(&runtime->ceval, tstate);
246278
PyThread_exit_thread();
247279
}
248280
}
@@ -280,13 +312,14 @@ void
280312
PyEval_AcquireLock(void)
281313
{
282314
_PyRuntimeState *runtime = &_PyRuntime;
283-
struct _ceval_runtime_state *ceval = &runtime->ceval;
284315
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
285-
if (tstate == NULL) {
286-
Py_FatalError("current thread state is NULL");
287-
}
288-
take_gil(ceval, tstate);
316+
ensure_tstate_not_null(__func__, tstate);
317+
289318
exit_thread_if_finalizing(tstate);
319+
assert(is_tstate_valid(tstate));
320+
321+
struct _ceval_runtime_state *ceval = &runtime->ceval;
322+
take_gil(ceval, tstate);
290323
}
291324

292325
void
@@ -304,15 +337,18 @@ PyEval_ReleaseLock(void)
304337
void
305338
PyEval_AcquireThread(PyThreadState *tstate)
306339
{
307-
assert(tstate != NULL);
340+
ensure_tstate_not_null(__func__, tstate);
341+
342+
exit_thread_if_finalizing(tstate);
343+
assert(is_tstate_valid(tstate));
308344

309345
_PyRuntimeState *runtime = tstate->interp->runtime;
310346
struct _ceval_runtime_state *ceval = &runtime->ceval;
311347

312348
/* Check someone has called PyEval_InitThreads() to create the lock */
313349
assert(gil_created(&ceval->gil));
350+
314351
take_gil(ceval, tstate);
315-
exit_thread_if_finalizing(tstate);
316352
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
317353
Py_FatalError("non-NULL old thread state");
318354
}
@@ -344,8 +380,9 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime)
344380
return;
345381
}
346382
recreate_gil(&ceval->gil);
347-
PyThreadState *current_tstate = _PyRuntimeState_GetThreadState(runtime);
348-
take_gil(ceval, current_tstate);
383+
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
384+
ensure_tstate_not_null(__func__, tstate);
385+
take_gil(ceval, tstate);
349386

350387
struct _pending_calls *pending = &ceval->pending;
351388
pending->lock = PyThread_allocate_lock();
@@ -354,7 +391,7 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime)
354391
}
355392

356393
/* Destroy all threads except the current one */
357-
_PyThreadState_DeleteExcept(runtime, current_tstate);
394+
_PyThreadState_DeleteExcept(runtime, tstate);
358395
}
359396

360397
/* This function is used to signal that async exceptions are waiting to be
@@ -383,16 +420,16 @@ PyEval_SaveThread(void)
383420
void
384421
PyEval_RestoreThread(PyThreadState *tstate)
385422
{
386-
assert(tstate != NULL);
423+
ensure_tstate_not_null(__func__, tstate);
424+
425+
exit_thread_if_finalizing(tstate);
426+
assert(is_tstate_valid(tstate));
387427

388428
_PyRuntimeState *runtime = tstate->interp->runtime;
389429
struct _ceval_runtime_state *ceval = &runtime->ceval;
390430
assert(gil_created(&ceval->gil));
391431

392-
int err = errno;
393432
take_gil(ceval, tstate);
394-
exit_thread_if_finalizing(tstate);
395-
errno = err;
396433

397434
_PyThreadState_Swap(&runtime->gilstate, tstate);
398435
}
@@ -750,11 +787,14 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
750787
PyObject **fastlocals, **freevars;
751788
PyObject *retval = NULL; /* Return value */
752789
_PyRuntimeState * const runtime = &_PyRuntime;
753-
PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime);
754790
struct _ceval_runtime_state * const ceval = &runtime->ceval;
755791
_Py_atomic_int * const eval_breaker = &ceval->eval_breaker;
756792
PyCodeObject *co;
757793

794+
PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime);
795+
ensure_tstate_not_null(__func__, tstate);
796+
assert(is_tstate_valid(tstate));
797+
758798
/* when tracing we set things up so that
759799
760800
not (instr_lb <= current_bytecode_offset < instr_ub)
@@ -1242,11 +1282,11 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
12421282

12431283
/* Other threads may run now */
12441284

1245-
take_gil(ceval, tstate);
1246-
12471285
/* Check if we should make a quick exit. */
12481286
exit_thread_if_finalizing(tstate);
12491287

1288+
take_gil(ceval, tstate);
1289+
12501290
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
12511291
Py_FatalError("orphan tstate");
12521292
}

Python/ceval_gil.h

+7-4
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,17 @@ drop_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
180180
#endif
181181
}
182182

183+
/* Take the GIL.
184+
185+
The function saves errno at entry and restores its value at exit.
186+
187+
tstate must be non-NULL. */
183188
static void
184189
take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
185190
{
186-
if (tstate == NULL) {
187-
Py_FatalError("take_gil: NULL tstate");
188-
}
191+
int err = errno;
189192

190193
struct _gil_runtime_state *gil = &ceval->gil;
191-
int err = errno;
192194
MUTEX_LOCK(gil->mutex);
193195

194196
if (!_Py_atomic_load_relaxed(&gil->locked)) {
@@ -240,6 +242,7 @@ take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
240242
}
241243

242244
MUTEX_UNLOCK(gil->mutex);
245+
243246
errno = err;
244247
}
245248

Python/pylifecycle.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1364,8 +1364,8 @@ Py_FinalizeEx(void)
13641364
int malloc_stats = interp->config.malloc_stats;
13651365
#endif
13661366

1367-
/* Remaining threads (e.g. daemon threads) will automatically exit
1368-
after taking the GIL (in PyEval_RestoreThread()). */
1367+
/* Remaining daemon threads will automatically exit
1368+
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
13691369
_PyRuntimeState_SetFinalizing(runtime, tstate);
13701370
runtime->initialized = 0;
13711371
runtime->core_initialized = 0;

0 commit comments

Comments
 (0)