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

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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
Original file line numberDiff line numberDiff line change
@@ -499,42 +499,66 @@ PyEval_ThreadsInitialized(void)
499499
return _PyEval_ThreadsInitialized();
50 10000 0500
}
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
/* 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