Skip to content

Commit 85f5a69

Browse files
authored
bpo-39877: Refactor take_gil() function (GH-18885)
* Remove ceval parameter of take_gil(): get it from tstate. * Move exit_thread_if_finalizing() call inside take_gil(). Replace exit_thread_if_finalizing() with tstate_must_exit(): the caller is now responsible to call PyThread_exit_thread(). * Move is_tstate_valid() assertion inside take_gil(). Remove is_tstate_valid(): inline code into take_gil(). * Move gil_created() assertion inside take_gil().
1 parent 363fab8 commit 85f5a69

File tree

2 files changed

+53
-66
lines changed

2 files changed

+53
-66
lines changed

Python/ceval.c

+13-65
Original file line numberDiff line numberDiff line change
@@ -198,16 +198,6 @@ ensure_tstate_not_null(const char *func, PyThreadState *tstate)
198198
}
199199

200200

201-
#ifndef NDEBUG
202-
static int is_tstate_valid(PyThreadState *tstate)
203-
{
204-
assert(!_PyMem_IsPtrFreed(tstate));
205-
assert(!_PyMem_IsPtrFreed(tstate->interp));
206-
return 1;
207-
}
208-
#endif
209-
210-
211201
int
212202
PyEval_ThreadsInitialized(void)
213203
{
@@ -230,13 +220,15 @@ _PyEval_InitThreads(PyThreadState *tstate)
230220

231221
PyThread_init_thread();
232222
create_gil(gil);
233-
take_gil(ceval, tstate);
223+
224+
take_gil(tstate);
234225

235226
struct _pending_calls *pending = &ceval->pending;
236227
pending->lock = PyThread_allocate_lock();
237228
if (pending->lock == NULL) {
238229
return _PyStatus_NO_MEMORY();
239230
}
231+
240232
return _PyStatus_OK();
241233
}
242234

@@ -269,30 +261,6 @@ _PyEval_FiniThreads(struct _ceval_runtime_state *ceval)
269261
}
270262
}
271263

272-
/* This function is designed to exit daemon threads immediately rather than
273-
taking the GIL if Py_Finalize() has been called.
274-
275-
The caller must *not* hold the GIL, since this function does not release
276-
the GIL before exiting the thread.
277-
278-
When this function is called by a daemon thread after Py_Finalize() has been
279-
called, the GIL does no longer exist.
280-
281-
tstate must be non-NULL. */
282-
static inline void
283-
exit_thread_if_finalizing(PyThreadState *tstate)
284-
{
285-
/* bpo-39877: Access _PyRuntime directly rather than using
286-
tstate->interp->runtime to support calls from Python daemon threads.
287-
After Py_Finalize() has been called, tstate can be a dangling pointer:
288-
point to PyThreadState freed memory. */
289-
_PyRuntimeState *runtime = &_PyRuntime;
290-
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
291-
if (finalizing != NULL && finalizing != tstate) {
292-
PyThread_exit_thread();
293-
}
294-
}
295-
296264
void
297265
_PyEval_Fini(void)
298266
{
@@ -329,11 +297,7 @@ PyEval_AcquireLock(void)
329297
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
330298
ensure_tstate_not_null(__func__, tstate);
331299

332-
exit_thread_if_finalizing(tstate);
333-
assert(is_tstate_valid(tstate));
334-
335-
struct _ceval_runtime_state *ceval = &runtime->ceval;
336-
take_gil(ceval, tstate);
300+
take_gil(tstate);
337301
}
338302

339303
void
@@ -353,17 +317,10 @@ PyEval_AcquireThread(PyThreadState *tstate)
353317
{
354318
ensure_tstate_not_null(__func__, tstate);
355319

356-
exit_thread_if_finalizing(tstate);
357-
assert(is_tstate_valid(tstate));
358-
359-
_PyRuntimeState *runtime = tstate->interp->runtime;
360-
struct _ceval_runtime_state *ceval = &runtime->ceval;
361-
362-
/* Check that _PyEval_InitThreads() was called to create the lock */
363-
assert(gil_created(&ceval->gil));
320+
take_gil(tstate);
364321

365-
take_gil(ceval, tstate);
366-
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
322+
struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
323+
if (_PyThreadState_Swap(gilstate, tstate) != NULL) {
367324
Py_FatalError("non-NULL old thread state");
368325
}
369326
}
@@ -396,7 +353,8 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime)
396353
recreate_gil(&ceval->gil);
397354
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
398355
ensure_tstate_not_null(__func__, tstate);
399-
take_gil(ceval, tstate);
356+
357+
take_gil(tstate);
400358

401359
struct _pending_calls *pending = &ceval->pending;
402360
pending->lock = PyThread_allocate_lock();
@@ -436,16 +394,10 @@ PyEval_RestoreThread(PyThreadState *tstate)
436394
{
437395
ensure_tstate_not_null(__func__, tstate);
438396

439-
exit_thread_if_finalizing(tstate);
440-
assert(is_tstate_valid(tstate));
397+
take_gil(tstate);
441398

442-
_PyRuntimeState *runtime = tstate->interp->runtime;
443-
struct _ceval_runtime_state *ceval = &runtime->ceval;
444-
assert(gil_created(&ceval->gil));
445-
446-
take_gil(ceval, tstate);
447-
448-
_PyThreadState_Swap(&runtime->gilstate, tstate);
399+
struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
400+
_PyThreadState_Swap(gilstate, tstate);
449401
}
450402

451403

@@ -805,7 +757,6 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
805757

806758
PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime);
807759
ensure_tstate_not_null(__func__, tstate);
808-
assert(is_tstate_valid(tstate));
809760

810761
/* when tracing we set things up so that
811762
@@ -1294,10 +1245,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
12941245

12951246
/* Other threads may run now */
12961247

1297-
/* Check if we should make a quick exit. */
1298-
exit_thread_if_finalizing(tstate);
1299-
1300-
take_gil(ceval, tstate);
1248+
take_gil(tstate);
13011249

13021250
if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
13031251
Py_FatalError("orphan tstate");

Python/ceval_gil.h

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

183+
184+
/* Check if a Python thread must exit immediately, rather than taking the GIL
185+
if Py_Finalize() has been called.
186+
187+
When this function is called by a daemon thread after Py_Finalize() has been
188+
called, the GIL does no longer exist.
189+
190+
tstate must be non-NULL. */
191+
static inline int
192+
tstate_must_exit(PyThreadState *tstate)
193+
{
194+
/* bpo-39877: Access _PyRuntime directly rather than using
195+
tstate->interp->runtime to support calls from Python daemon threads.
196+
After Py_Finalize() has been called, tstate can be a dangling pointer:
197+
point to PyThreadState freed memory. */
198+
_PyRuntimeState *runtime = &_PyRuntime;
199+
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
200+
return (finalizing != NULL && finalizing != tstate);
201+
}
202+
203+
183204
/* Take the GIL.
184205
185206
The function saves errno at entry and restores its value at exit.
186207
187208
tstate must be non-NULL. */
188209
static void
189-
take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
210+
take_gil(PyThreadState *tstate)
190211
{
191212
int err = errno;
192213

214+
assert(tstate != NULL);
215+
216+
/* Check if we should make a quick exit. */
217+
if (tstate_must_exit(tstate)) {
218+
PyThread_exit_thread();
219+
}
220+
221+
/* Ensure that tstate is valid: sanity check for PyEval_AcquireThread() and
222+
PyEval_RestoreThread(). Detect if tstate memory was freed. */
223+
assert(!_PyMem_IsPtrFreed(tstate));
224+
assert(!_PyMem_IsPtrFreed(tstate->interp));
225+
226+
struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval;
193227
struct _gil_runtime_state *gil = &ceval->gil;
228+
229+
/* Check that _PyEval_InitThreads() was called to create the lock */
230+
assert(gil_created(gil));
231+
194232
MUTEX_LOCK(gil->mutex);
195233

196234
if (!_Py_atomic_load_relaxed(&gil->locked)) {
@@ -215,6 +253,7 @@ take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate)
215253
SET_GIL_DROP_REQUEST(ceval);
216254
}
217255
}
256+
218257
_ready:
219258
#ifdef FORCE_SWITCHING
220259
/* This mutex must be taken before modifying gil->last_holder:

0 commit comments

Comments
 (0)