Skip to content

gh-112723: Call PyThreadState_Clear() from the correct interpreter #112776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 13, 2023
Merged

gh-112723: Call PyThreadState_Clear() from the correct interpreter #112776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ _PyEval_Vector(PyThreadState *tstate,
PyObject *kwnames);

extern int _PyEval_ThreadsInitialized(void);
extern PyStatus _PyEval_InitGIL(PyThreadState *tstate, int own_gil);
extern void _PyEval_InitGIL(PyThreadState *tstate, int own_gil);
extern void _PyEval_FiniGIL(PyInterpreterState *interp);

extern void _PyEval_AcquireLock(PyThreadState *tstate);
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ extern void _PyArg_Fini(void);
extern void _Py_FinalizeAllocatedBlocks(_PyRuntimeState *);

extern PyStatus _PyGILState_Init(PyInterpreterState *interp);
extern PyStatus _PyGILState_SetTstate(PyThreadState *tstate);
extern void _PyGILState_SetTstate(PyThreadState *tstate);
extern void _PyGILState_Fini(PyInterpreterState *interp);

extern void _PyGC_DumpShutdownStats(PyInterpreterState *interp);
Expand Down
2 changes: 2 additions & 0 deletions Modules/_xxsubinterpretersmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,9 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds)
return NULL;
}

PyThreadState_Swap(tstate);
PyThreadState_Clear(tstate);
PyThreadState_Swap(save_tstate);
PyThreadState_Delete(tstate);

_PyInterpreterState_RequireIDRef(interp, 1);
Expand Down
4 changes: 1 addition & 3 deletions Python/ceval_gil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ init_own_gil(PyInterpreterState *interp, struct _gil_runtime_state *gil)
interp->ceval.own_gil = 1;
}

PyStatus
void
_PyEval_InitGIL(PyThreadState *tstate, int own_gil)
{
assert(tstate->interp->ceval.gil == NULL);
Expand All @@ -471,8 +471,6 @@ _PyEval_InitGIL(PyThreadState *tstate, int own_gil)

// Lock the GIL and mark the current thread as attached.
_PyThreadState_Attach(tstate);

return _PyStatus_OK();
}

void
Expand Down
72 changes: 26 additions & 46 deletions Python/pylifecycle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,44 +576,33 @@ init_interp_settings(PyInterpreterState *interp,
interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS;
}

/* We check "gil" in init_interp_create_gil(). */
switch (config->gil) {
case PyInterpreterConfig_DEFAULT_GIL: break;
case PyInterpreterConfig_SHARED_GIL: break;
case PyInterpreterConfig_OWN_GIL: break;
default:
return _PyStatus_ERR("invalid interpreter config 'gil' value");
}

return _PyStatus_OK();
}


static PyStatus
static void
init_interp_create_gil(PyThreadState *tstate, int gil)
{
PyStatus status;

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

/* Auto-thread-state API */
status = _PyGILState_SetTstate(tstate);
if (_PyStatus_EXCEPTION(status)) {
return status;
}
_PyGILState_SetTstate(tstate);

int own_gil;
switch (gil) {
case PyInterpreterConfig_DEFAULT_GIL: own_gil = 0; break;
case PyInterpreterConfig_SHARED_GIL: own_gil = 0; break;
case PyInterpreterConfig_OWN_GIL: own_gil = 1; break;
default:
return _PyStatus_ERR("invalid interpreter config 'gil' value");
}
int own_gil = (gil == PyInterpreterConfig_OWN_GIL);

/* Create the GIL and take it */
status = _PyEval_InitGIL(tstate, own_gil);
if (_PyStatus_EXCEPTION(status)) {
return status;
}

return _PyStatus_OK();
_PyEval_InitGIL(tstate, own_gil);
}


Expand Down Expand Up @@ -657,10 +646,7 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
}
_PyThreadState_Bind(tstate);

status = init_interp_create_gil(tstate, config.gil);
if (_PyStatus_EXCEPTION(status)) {
return status;
}
init_interp_create_gil(tstate, config.gil);

*tstate_p = tstate;
return _PyStatus_OK();
Expand Down Expand Up @@ -2087,28 +2073,21 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
return _PyStatus_OK();
}

PyThreadState *tstate = _PyThreadState_New(interp,
_PyThreadState_WHENCE_INTERP);
if (tstate == NULL) {
PyInterpreterState_Delete(interp);
*tstate_p = NULL;
return _PyStatus_OK();
}
_PyThreadState_Bind(tstate);

// XXX Might new_interpreter() have been called without the GIL held?
PyThreadState *save_tstate = _PyThreadState_GET();
int has_gil = 0;
PyThreadState *tstate = NULL;

/* From this point until the init_interp_create_gil() call,
we must not do anything that requires that the GIL be held
(or otherwise exist). That applies whether or not the new
interpreter has its own GIL (e.g. the main interpreter). */
if (save_tstate != NULL) {
_PyThreadState_Detach(save_tstate);
}

/* Copy the current interpreter config into the new interpreter */
const PyConfig *src_config;
if (save_tstate != NULL) {
// XXX Might new_interpreter() have been called without the GIL held?
_PyThreadState_Detach(save_tstate);
src_config = _PyInterpreterState_GetConfig(save_tstate->interp);
}
else
Expand All @@ -2130,11 +2109,14 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
goto error;
}

status = init_interp_create_gil(tstate, config->gil);
if (_PyStatus_EXCEPTION(status)) {
tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_INTERP);
if (tstate == NULL) {
status = _PyStatus_NO_MEMORY();
goto error;
}
has_gil = 1;

_PyThreadState_Bind(tstate);
init_interp_create_gil(tstate, config->gil);

/* No objects have been created yet. */

Expand All @@ -2153,16 +2135,14 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)

error:
*tstate_p = NULL;

/* Oops, it didn't work. Undo it all. */
if (has_gil) {
if (tstate != NULL) {
PyThreadState_Clear(tstate);
_PyThreadState_Detach(tstate);
PyThreadState_Delete(tstate);
}
if (save_tstate != NULL) {
_PyThreadState_Attach(save_tstate);
}
PyThreadState_Clear(tstate);
PyThreadState_Delete(tstate);
PyInterpreterState_Delete(interp);

return status;
Expand Down
7 changes: 3 additions & 4 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,7 @@ void
PyThreadState_Clear(PyThreadState *tstate)
{
assert(tstate->_status.initialized && !tstate->_status.cleared);
assert(current_fast_get(&_PyRuntime)->interp == tstate->interp);
// XXX assert(!tstate->_status.bound || tstate->_status.unbound);
tstate->_status.finalizing = 1; // just in case

Expand Down Expand Up @@ -2243,7 +2244,7 @@ _PyGILState_Fini(PyInterpreterState *interp)


// XXX Drop this.
PyStatus
void
_PyGILState_SetTstate(PyThreadState *tstate)
{
/* must init with valid states */
Expand All @@ -2253,7 +2254,7 @@ _PyGILState_SetTstate(PyThreadState *tstate)
if (!_Py_IsMainInterpreter(tstate->interp)) {
/* Currently, PyGILState is shared by all interpreters. The main
* interpreter is responsible to initialize it. */
return _PyStatus_OK();
return;
}

#ifndef NDEBUG
Expand All @@ -2263,8 +2264,6 @@ _PyGILState_SetTstate(PyThreadState *tstate)
assert(gilstate_tss_get(runtime) == tstate);
assert(tstate->gilstate_counter == 1);
#endif

return _PyStatus_OK();
}

PyInterpreterState *
Expand Down