8000 [3.12] gh-109860: Use a New Thread State When Switching Interpreters, When Necessary (gh-110245) by ericsnowcurrently · Pull Request #110709 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

[3.12] gh-109860: Use a New Thread State When Switching Interpreters, When Necessary (gh-110245) #110709

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.

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

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
[3.12] gh-109860: Use a New Thread State When Switching Interpreters,…
… When Necessary (gh-110245)

In a few places we switch to another interpreter without knowing if it has a thread state associated with the current thread.  For the main interpreter there wasn't much of a problem, but for subinterpreters we were *mostly* okay re-using the tstate created with the interpreter (located via PyInterpreterState_ThreadHead()).  There was a good chance that tstate wasn't actually in use by another thread.

However, there are no guarantees of that.  Furthermore, re-using an already used tstate is currently fragile.  To address this, now we create a new thread state in each of those places and use it.

One consequence of this change is that PyInterpreterState_ThreadHead() may not return NULL (though that won't happen for the main interpreter).

(cherry-picked from commit f5198b0)
  • Loading branch information
ericsnowcurrently committed Oct 12, 2023
commit 9a3588e11e2cd0ec141bbfe19d60c62c1e0bdaaa
9 changes: 9 additions & 0 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ struct _ts {
/* padding to align to 4 bytes */
unsigned int :24;
} _status;
#ifdef Py_BUILD_CORE
# define _PyThreadState_WHENCE_NOTSET -1
# define _PyThreadState_WHENCE_UNKNOWN 0
# define _PyThreadState_WHENCE_INTERP 1
# define _PyThreadState_WHENCE_THREADING 2
# define _PyThreadState_WHENCE_GILSTATE 3
# define _PyThreadState_WHENCE_EXEC 4
#endif
int _whence;

int py_recursion_remaining;
int py_recursion_limit;
Expand Down
5 changes: 4 additions & 1 deletion Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ _Py_IsMainInterpreterFinalizing(PyInterpreterState *interp)
PyAPI_FUNC(int) _PyInterpreterState_SetRunningMain(PyInterpreterState *);
PyAPI_FUNC(void) _PyInterpreterState_SetNotRunningMain(PyInterpreterState *);
PyAPI_FUNC(int) _PyInterpreterState_IsRunningMain(PyInterpreterState *);
PyAPI_FUNC(int) _PyInterpreterState_FailIfRunningMain(PyInterpreterState *);


static inline const PyConfig *
Expand Down Expand Up @@ -132,7 +133,9 @@ static inline PyInterpreterState* _PyInterpreterState_GET(void) {

// PyThreadState functions

PyAPI_FUNC(PyThreadState *) _PyThreadState_New(PyInterpreterState *interp);
PyAPI_FUNC(PyThreadState *) _PyThreadState_New(
PyInterpreterState *interp,
int whence);
PyAPI_FUNC(void) _PyThreadState_Bind(PyThreadState *tstate);
// We keep this around exclusively for stable ABI compatibility.
PyAPI_FUNC(void) _PyThreadState_Init(
Expand Down
1 change: 1 addition & 0 deletions 10000 Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ extern PyTypeObject _PyExc_MemoryError;

#define _PyThreadState_INIT \
{ \
._whence = _PyThreadState_WHENCE_NOTSET, \
.py_recursion_limit = Py_DEFAULT_RECURSION_LIMIT, \
.context_ver = 1, \
}
Expand Down
7 changes: 5 additions & 2 deletions Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1585,8 +1585,11 @@ def _shutdown():
# The main thread isn't finished yet, so its thread state lock can't
# have been released.
assert tlock is not None
assert tlock.locked()
tlock.release()
if tlock.locked():
# It should have been released already by
# _PyInterpreterState_SetNotRunningMain(), but there may be
# embedders that aren't calling that yet.
tlock.release()
_main_thread._stop()
else:
# bpo-1596321: _shutdown() must be called in the main thread.
Expand Down
2 changes: 1 addition & 1 deletion Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
if (boot == NULL) {
return PyErr_NoMemory();
}
boot->tstate = _PyThreadState_New(interp);
boot->tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_THREADING);
if (boot->tstate == NULL) {
PyMem_RawFree(boot);
if (!PyErr_Occurred()) {
Expand Down
111 changes: 67 additions & 44 deletions Modules/_xxsubinterpretersmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ _sharedns_apply(_sharedns *shared, PyObject *ns)
// of the exception in the calling interpreter.

typedef struct _sharedexception {
PyInterpreterState *interp;
#define ERR_NOT_SET 0
#define ERR_NO_MEMORY 1
#define ERR_ALREADY_RUNNING 2
int code;
const char *name;
const char *msg;
} _sharedexception;
Expand All @@ -264,14 +269,26 @@ _sharedexception_clear(_sharedexception *exc)
}

static const char *
_sharedexception_bind(PyObject *exc, _sharedexception *sharedexc)
_sharedexception_bind(PyObject *exc, int code, _sharedexception *sharedexc)
{
if (sharedexc->interp == NULL) {
sharedexc->interp = PyInterpreterState_Get();
}

if (code != ERR_NOT_SET) {
assert(exc == NULL);
assert(code > 0);
sharedexc->code = code;
return NULL;
}

assert(exc != NULL);
const char *failure = NULL;

PyObject *nameobj = PyUnicode_FromFormat("%S", Py_TYPE(exc));
if (nameobj == NULL) {
failure = "unable to format exception type name";
code = ERR_NO_MEMORY;
goto error;
}
sharedexc->name = _copy_raw_string(nameobj);
Expand All @@ -282,13 +299,15 @@ _sharedexception_bind(PyObject *exc, _sharedexception *sharedexc)
} else {
failure = "unable to encode and copy exception type name";
}
code = ERR_NO_MEMORY;
goto error;
}

if (exc != NULL) {
PyObject *msgobj = PyUnicode_FromFormat("%S", exc);
if (msgobj == NULL) {
failure = "unable to format exception message";
code = ERR_NO_MEMORY;
goto error;
}
sharedexc->msg = _copy_raw_string(msgobj);
Expand All @@ -299,6 +318,7 @@ _sharedexception_bind(PyObject *exc, _sharedexception *sharedexc)
} else {
failure = "unable to encode and copy exception message";
}
code = ERR_NO_MEMORY;
goto error;
}
}
Expand All @@ -309,14 +329,18 @@ _sharedexception_bind(PyObject *exc, _sharedexception *sharedexc)
assert(failure != NULL);
PyErr_Clear();
_sharedexception_clear(sharedexc);
*sharedexc = no_exception;
*sharedexc = (_sharedexception){
.interp = sharedexc->interp,
.code = code,
};
return failure;
}

static void
_sharedexception_apply(_sharedexception *exc, PyObject *wrapperclass)
{
if (exc->name != NULL) {
assert(exc->code == ERR_NOT_SET);
if (exc->msg != NULL) {
PyErr_Format(wrapperclass, "%s: %s", exc->name, exc->msg);
}
Expand All @@ -325,9 +349,19 @@ _sharedexception_apply(_sharedexception *exc, PyObject *wrapperclass)
}
}
else if (exc->msg != NULL) {
assert(exc->code == ERR_NOT_SET);
PyErr_SetString(wrapperclass, exc->msg);
}
else if (exc->code == ERR_NO_MEMORY) {
PyErr_NoMemory();
}
else if (exc->code == ERR_ALREADY_RUNNING) {
assert(exc->interp != NULL);
assert(_PyInterpreterState_IsRunningMain(exc->interp));
_PyInterpreterState_FailIfRunningMain(exc->interp);
}
else {
assert(exc->code == ERR_NOT_SET);
PyErr_SetNone(wrapperclass);
}
}
Expand Down Expand Up @@ -363,9 +397,16 @@ static int
_run_script(PyInterpreterState *interp, const char *codestr,
_sharedns *shared, _sharedexception *sharedexc)
{
int errcode = ERR_NOT_SET;

if (_PyInterpreterState_SetRunningMain(interp) < 0) {
// We skip going through the shared exception.
return -1;
assert(PyErr_Occurred());
// In the case where we didn't switch interpreters, it would
// be more efficient to leave the exception in place and return
// immediately. However, life is simpler if we don't.
PyErr_Clear();
errcode = ERR_ALREADY_RUNNING;
goto error;
}

PyObject *excval = NULL;
Expand Down Expand Up @@ -404,16 +445,17 @@ _run_script(PyInterpreterState *interp, const char *codestr,

error:
excval = PyErr_GetRaisedException();
const char *failure = _sharedexception_bind(excval, sharedexc);
const char *failure = _sharedexception_bind(excval, errcode, sharedexc);
if (failure != NULL) {
fprintf(stderr,
"RunFailedError: script raised an uncaught exception (%s)",
failure);
PyErr_Clear();
}
Py_XDECREF(excval);
if (errcode != ERR_ALREADY_RUNNING) {
_PyInterpreterState_SetNotRunningMain(interp);
}
assert(!PyErr_Occurred());
_PyInterpreterState_SetNotRunningMain(interp);
return -1;
}

Expand All @@ -422,6 +464,7 @@ _run_script_in_interpreter(PyObject *mod, PyInterpreterState *interp,
const char *codestr, PyObject *shareables)
{
module_state *state = get_module_state(mod);
assert(state != NULL);

_sharedns *shared = _get_shared_ns(shareables);
if (shared == NULL && PyErr_Occurred()) {
Expand All @@ -430,50 +473,30 @@ _run_script_in_interpreter(PyObject *mod, PyInterpreterState *interp,

// Switch to interpreter.
PyThreadState *save_tstate = NULL;
PyThreadState *tstate = NULL;
if (interp != PyInterpreterState_Get()) {
// XXX gh-109860: Using the "head" thread isn't strictly correct.
PyThreadState *tstate = PyInterpreterState_ThreadHead(interp);
assert(tstate != NULL);
// Hack (until gh-109860): The interpreter's initial thread state
// is least likely to break.
while(tstate->next != NULL) {
tstate = tstate->next;
}
// We must do this check before switching interpreters, so any
// exception gets raised in the right one.
// XXX gh-109860: Drop this redundant check once we stop
// re-using tstates that might already be in use.
if (_PyInterpreterState_IsRunningMain(interp)) {
PyErr_SetString(PyExc_RuntimeError,
"interpreter already running");
if (shared != NULL) {
_sharedns_free(shared);
}
return -1;
}
tstate = PyThreadState_New(interp);
tstate->_whence = _PyThreadState_WHENCE_EXEC;
// XXX Possible GILState issues?
save_tstate = PyThreadState_Swap(tstate);
}

// Run the script.
_sharedexception exc = {NULL, NULL};
_sharedexception exc = (_sharedexception){ .interp = interp };
int result = _run_script(interp, codestr, shared, &exc);

// Switch back.
if (save_tstate != NULL) {
PyThreadState_Clear(tstate);
PyThreadState_Swap(save_tstate);
PyThreadState_Delete(tstate);
}

// Propagate any exception out to the caller.
if (exc.name != NULL) {
assert(state != NULL);
if (result < 0) {
assert(!PyErr_Occurred());
_sharedexception_apply(&exc, state->RunFailedError);
}
else if (result != 0) {
if (!PyErr_Occurred()) {
// We were unable to allocate a shared exception.
PyErr_NoMemory();
}
assert(PyErr_Occurred());
}

if (shared != NULL) {
Expand Down Expand Up @@ -503,6 +526,7 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds)
const PyInterpreterConfig config = isolated
? (PyInterpreterConfig)_PyInterpreterConfig_INIT
: (PyInterpreterConfig)_PyInterpreterConfig_LEGACY_INIT;

// XXX Possible GILState issues?
PyThreadState *tstate = NULL;
PyStatus status = Py_NewInterpreterFromConfig(&tstate, &config);
Expand All @@ -518,6 +542,7 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds)
return NULL;
}
assert(tstate != NULL);

PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate);
PyObject *idobj = _PyInterpreterState_GetIDObject(interp);
if (idobj == NULL) {
Expand All @@ -527,6 +552,10 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds)
PyThreadState_Swap(save_tstate);
return NULL;
}

PyThreadState_Clear(tstate);
PyThreadState_Delete(tstate);

_PyInterpreterState_RequireIDRef(interp, 1);
return idobj;
}
Expand Down Expand Up @@ -574,14 +603,8 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds)
}

// Destroy the interpreter.
// XXX gh-109860: Using the "head" thread isn't strictly correct.
PyThreadState *tstate = PyInterpreterState_ThreadHead(interp);
assert(tstate != NULL);
// Hack (until gh-109860): The interpreter's initial thread state
// is least likely to break.
while(tstate->next != NULL) {
tstate = tstate->next;
}
PyThreadState *tstate = PyThreadState_New(interp);
tstate->_whence = _PyThreadState_WHENCE_INTERP;
// XXX Possible GILState issues?
PyThreadState *save_tstate = PyThreadState_Swap(tstate);
Py_EndInterpreter(tstate);
Expand Down
6 changes: 4 additions & 2 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,8 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
return status;
}

PyThreadState *tstate = _PyThreadState_New(interp);
PyThreadState *tstate = _PyThreadState_New(interp,
_PyThreadState_WHENCE_INTERP);
if (tstate == NULL) {
return _PyStatus_ERR("can't make first thread");
}
Expand Down Expand Up @@ -2051,7 +2052,8 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
return _PyStatus_OK();
}

PyThreadState *tstate = _PyThreadState_New(interp);
PyThreadState *tstate = _PyThreadState_New(interp,
_PyThreadState_WHENCE_INTERP);
if (tstate == NULL) {
PyInterpreterState_Delete(interp);
*tstate_p = NULL;
Expand Down
Loading
0