8000 gh-99113: Make Sure the GIL is Acquired at the Right Places (gh-104208) · python/cpython@92d8bff · GitHub
[go: up one dir, main page]
Skip to content

Commit 92d8bff

Browse files
ericsnowcurrentlyericsnowcurrently
authored
gh-99113: Make Sure the GIL is Acquired at the Right Places (gh-104208)
This is a pre-requisite for a per-interpreter GIL. Without it this change isn't strictly necessary. However, there is no real downside otherwise.
1 parent fff193b commit 92d8bff

File tree

4 files changed

+113
-40
lines changed

4 files changed

+113
-40
lines changed

‎Include/internal/pycore_ceval.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ extern int _PyEval_ThreadsInitialized(void);
100100
extern PyStatus _PyEval_InitGIL(PyThreadState *tstate, int own_gil);
101101
extern void _PyEval_FiniGIL(PyInterpreterState *interp);
102102

103+
extern void _PyEval_AcquireLock(PyThreadState *tstate);
103104
extern void _PyEval_ReleaseLock(PyThreadState *tstate);
105+
extern PyThreadState * _PyThreadState_SwapNoGIL(PyThreadState *);
104106

105107
extern void _PyEval_DeactivateOpCache(void);
106108

‎Python/ceval_gil.c

Lines changed: 57 additions & 27 deletions
6D47
Original file line numberDiff line numberDiff line change
@@ -499,42 +499,66 @@ PyEval_ThreadsInitialized(void)
499499
return _PyEval_ThreadsInitialized();
500500
}
501501

502+
static inline int
503+
current_thread_holds_gil(struct _gil_runtime_state *gil, PyThreadState *tstate)
504+
{
505+
if (((PyThreadState*)_Py_atomic_load_relaxed(&gil->last_holder)) != tstate) {
506+
return 0;
507+
}
508+
return _Py_atomic_load_relaxed(&gil->locked);
509+
}
510+
511+
static void
512+
init_shared_gil(PyInterpreterState *interp, struct _gil_runtime_state *gil)
513+
{
514+
assert(gil_created(gil));
515+
interp->ceval.gil = gil;
516+
interp->ceval.own_gil = 0;
517+
}
518+
519+
static void
520+
init_own_gil(PyInterpreterState *interp, struct _gil_runtime_state *gil)
521+
{
522+
assert(!gil_created(gil));
523+
create_gil(gil);
524+
assert(gil_created(gil));
525+
interp->ceval.gil = gil;
526+
interp->ceval.own_gil = 1;
527+
}
528+
502529
PyStatus
503530
_PyEval_InitGIL(PyThreadState *tstate, int own_gil)
504531
{
505532
assert(tstate->interp->ceval.gil == NULL);
533+
int locked;
506534
if (!own_gil) {
507535
PyInterpreterState *main_interp = _PyInterpreterState_Main();
508536
assert(tstate->interp != main_interp);
509537
struct _gil_runtime_state *gil = main_interp->ceval.gil;
510-
assert(gil_created(gil));
511-
tstate->interp->ceval.gil = gil;
512-
tstate->interp->ceval.own_gil = 0;
513-
return _PyStatus_OK();
538+
init_shared_gil(tstate->interp, gil);
539+
locked = current_thread_holds_gil(gil, tstate);
514540
}
515-
516541
/* XXX per-interpreter GIL */
517-
struct _gil_runtime_state *gil = &tstate->interp->runtime->ceval.gil;
518-
if (!_Py_IsMainInterpreter(tstate->interp)) {
542+
else if (!_Py_IsMainInterpreter(tstate->interp)) {
519543
/* 9E81 Currently, the GIL is shared by all interpreters,
520544
and only the main interpreter is responsible to create
521545
and destroy it. */
522-
assert(gil_created(gil));
523-
tstate->interp->ceval.gil = gil;
546+
struct _gil_runtime_state *main_gil = _PyInterpreterState_Main()->ceval.gil;
547+
init_shared_gil(tstate->interp, main_gil);
524548
// XXX For now we lie.
525549
tstate->interp->ceval.own_gil = 1;
526-
return _PyStatus_OK();
550+
locked = current_thread_holds_gil(main_gil, tstate);
551+
}
552+
else {
553+
PyThread_init_thread();
554+
// XXX per-interpreter GIL: switch to interp->_gil.
555+
init_own_gil(tstate->interp, &tstate->interp->runtime->ceval.gil);
556+
locked = 0;
557+
}
558+
if (!locked) {
559+
take_gil(tstate);
527560
}
528-
assert(own_gil);
529-
530-
assert(!gil_created(gil));
531561

532-
PyThread_init_thread();
533-
create_gil(gil);
534-
assert(gil_created(gil));
535-
tstate->interp->ceval.gil = gil;
536-
tstate->interp->ceval.own_gil = 1;
537-
take_gil(tstate);
538562
return _PyStatus_OK();
539563
}
540564

@@ -611,9 +635,17 @@ PyEval_ReleaseLock(void)
611635
drop_gil(ceval, tstate);
612636
}
613637

638+
void
639+
_PyEval_AcquireLock(PyThreadState *tstate)
640+
{
641+
_Py_EnsureTstateNotNULL(tstate);
642+
take_gil(tstate);
643+
}
644+
614645
void
615646
_PyEval_ReleaseLock(PyThreadState *tstate)
616647
{
648+
_Py_EnsureTstateNotNULL(tstate);
617649
struct _ceval_state *ceval = &tstate->interp->ceval;
618650
drop_gil(ceval, tstate);
619651
}
@@ -625,7 +657,7 @@ PyEval_AcquireThread(PyThreadState *tstate)
625657

626658
take_gil(tstate);
627659

628-
if (_PyThreadState_Swap(tstate->interp->runtime, tstate) != NULL) {
660+
if (_PyThreadState_SwapNoGIL(tstate) != NULL) {
629661
Py_FatalError("non-NULL old thread state");
630662
}
631663
}
@@ -635,8 +667,7 @@ PyEval_ReleaseThread(PyThreadState *tstate)
635667
{
636668
assert(is_tstate_valid(tstate));
637669

638-
_PyRuntimeState *runtime = tstate->interp->runtime;
639-
PyThreadState *new_tstate = _PyThreadState_Swap(runtime, NULL);
670+
PyThreadState *new_tstate = _PyThreadState_SwapNoGIL(NULL);
640671
if (new_tstate != tstate) {
641672
Py_FatalError("wrong thread state");
642673
}
@@ -684,8 +715,7 @@ _PyEval_SignalAsyncExc(PyInterpreterState *interp)
684715
PyThreadState *
685716
PyEval_SaveThread(void)
686717
{
687-
_PyRuntimeState *runtime = &_PyRuntime;
688-
PyThreadState *tstate = _PyThreadState_Swap(runtime, NULL);
718+
PyThreadState *tstate = _PyThreadState_SwapNoGIL(NULL);
689719
_Py_EnsureTstateNotNULL(tstate);
690720

691721
struct _ceval_state *ceval = &tstate->interp->ceval;
@@ -701,7 +731,7 @@ PyEval_RestoreThread(PyThreadState *tstate)
701731

702732
take_gil(tstate);
703733

704-
_PyThreadState_Swap(tstate->interp->runtime, tstate);
734+
_PyThreadState_SwapNoGIL(tstate);
705735
}
706736

707737

@@ -1005,7 +1035,7 @@ _Py_HandlePending(PyThreadState *tstate)
10051035
/* GIL drop request */
10061036
if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gil_drop_request)) {
10071037
/* Give another thread a chance */
1008-
if (_PyThreadState_Swap(runtime, NULL) != tstate) {
1038+
if (_PyThreadState_SwapNoGIL(NULL) != tstate) {
10091039
Py_FatalError("tstate mix-up");
10101040
}
10111041
drop_gil(interp_ceval_state, tstate);
@@ -1014,7 +1044,7 @@ _Py_HandlePending(PyThreadState *tstate)
10141044

10151045
take_gil(tstate);
10161046

1017-
if (_PyThreadState_Swap(runtime, tstate) != NULL) {
1047+
if (_PyThreadState_SwapNoGIL(tstate) != NULL) {
10181048
Py_FatalError("orphan tstate");
10191049
}
10201050
}

‎Python/pylifecycle.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ init_interp_create_gil(PyThreadState *tstate, int own_gil)
591591

592592
/* finalize_interp_delete() comment explains why _PyEval_FiniGIL() is
593593
only called here. */
594+
// XXX This is broken with a per-interpreter GIL.
594595
_PyEval_FiniGIL(tstate->interp);
595596

596597
/* Auto-thread-state API */
@@ -645,7 +646,8 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
645646
return _PyStatus_ERR("can't make first thread");
646647
}
647648
_PyThreadState_Bind(tstate);
648-
(void) PyThreadState_Swap(tstate);
649+
// XXX For now we do this before the GIL is created.
650+
(void) _PyThreadState_SwapNoGIL(tstate);
649651

650652
status = init_interp_create_gil(tstate, config.own_gil);
651653
if (_PyStatus_EXCEPTION(status)) {
@@ -2025,11 +2027,20 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
20252027
}
20262028
_PyThreadState_Bind(tstate);
20272029

2028-
PyThreadState *save_tstate = PyThreadState_Swap(tstate);
2030+
// XXX For now we do this before the GIL is created.
2031+
PyThreadState *save_tstate = _PyThreadState_SwapNoGIL(tstate);
2032+
int has_gil = 0;
2033+
2034+
/* From this point until the init_interp_create_gil() call,
2035+
we must not do anything that requires that the GIL be held
2036+
(or otherwise exist). That applies whether or not the new
2037+
interpreter has its own GIL (e.g. the main interpreter). */
20292038

20302039
/* Copy the current interpreter config into the new interpreter */
20312040
const PyConfig *src_config;
20322041
if (save_tstate != NULL) {
2042+
// XXX Might new_interpreter() have been called without the GIL held?
2043+
_PyEval_ReleaseLock(save_tstate);
20332044
src_config = _PyInterpreterState_GetConfig(save_tstate->interp);
20342045
}
20352046
else
@@ -2039,11 +2050,13 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
20392050
src_config = _PyInterpreterState_GetConfig(main_interp);
20402051
}
20412052

2053+
/* This does not require that the GIL be held. */
20422054
status = _PyConfig_Copy(&interp->config, src_config);
20432055
if (_PyStatus_EXCEPTION(status)) {
20442056
goto error;
20452057
}
20462058

2059+
/* This does not require that the GIL be held. */
20472060
status = init_interp_settings(interp, config);
20482061
if (_PyStatus_EXCEPTION(status)) {
20492062
goto error;
@@ -2053,6 +2066,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
20532066
if (_PyStatus_EXCEPTION(status)) {
20542067
goto error;
20552068
}
2069+
has_gil = 1;
20562070

20572071
status = pycore_interp_init(tstate);
20582072
if (_PyStatus_EXCEPTION(status)) {
@@ -2072,7 +2086,12 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
20722086

20732087
/* Oops, it didn't work. Undo it all. */
20742088
PyErr_PrintEx(0);
2075-
PyThreadState_Swap(save_tstate);
2089+
if (has_gil) {
2090+
PyThreadState_Swap(save_tstate);
2091+
}
2092+
else {
2093+
_PyThreadState_SwapNoGIL(save_tstate);
2094+
}
20762095
PyThreadState_Clear(tstate);
20772096
PyThreadState_Delete(tstate);
20782097
PyInterpreterState_Delete(interp);

‎Python/pystate.c

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,17 +1863,11 @@ PyThreadState_Get(void)
18631863
}
18641864

18651865

1866-
PyThreadState *
1867-
_PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
1866+
static void
1867+
_swap_thread_states(_PyRuntimeState *runtime,
1868+
PyThreadState *oldts, PyThreadState *newts)
18681869
{
1869-
#if defined(Py_DEBUG)
1870-
/* This can be called from PyEval_RestoreThread(). Similar
1871-
to it, we need to ensure errno doesn't change.
1872-
*/
1873-
int err = errno;
1874-
#endif
1875-
PyThreadState *oldts = current_fast_get(runtime);
1876-
1870+
// XXX Do this only if oldts != NULL?
18771871
current_fast_clear(runtime);
18781872

18791873
if (oldts != NULL) {
@@ -1887,13 +1881,41 @@ _PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
18871881
current_fast_set(runtime, newts);
18881882
tstate_activate(newts);
18891883
}
1884+
}
1885+
1886+
PyThreadState *
1887+
_PyThreadState_SwapNoGIL(PyThreadState *newts)
1888+
{
1889+
#if defined(Py_DEBUG)
1890+
/* This can be called from PyEval_RestoreThread(). Similar
1891+
to it, we need to ensure errno doesn't change.
1892+
*/
1893+
int err = errno;
1894+
#endif
1895+
1896+
PyThreadState *oldts = current_fast_get(&_PyRuntime);
1897+
_swap_thread_states(&_PyRuntime, oldts, newts);
18901898

18911899
#if defined(Py_DEBUG)
18921900
errno = err;
18931901
#endif
18941902
return oldts;
18951903
}
18961904

1905+
PyThreadState *
1906+
_PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
1907+
{
1908+
PyThreadState *oldts = current_fast_get(runtime);
1909+
if (oldts != NULL) {
1910+
_PyEval_ReleaseLock(oldts);
1911+
}
1912+
_swap_thread_states(runtime, oldts, newts);
1913+
if (newts != NULL) {
1914+
_PyEval_AcquireLock(newts);
1915+
}
1916+
return oldts;
1917+
}
1918+
18971919
PyThreadState *
18981920
PyThreadState_Swap(PyThreadState *newts)
18991921
{

0 commit comments

Comments
 (0)
0