Skip to content

gh-117953: Let update_global_state_for_extension() Caller Decide If Singlephase or Not #118193

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

Merged
Changes from all commits
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
113 changes: 93 additions & 20 deletions Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -1185,19 +1185,51 @@ is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *path)
return 0;
}

#ifndef NDEBUG
static bool
is_singlephase(PyModuleDef *def)
{
if (def == NULL) {
/* It must be a module created by reload_singlephase_extension()
* from m_copy. Ideally we'd do away with this case. */
return true;
}
else if (def->m_slots == NULL) {
return true;
}
else {
return false;
}
}
#endif


struct singlephase_global_update {
PyObject *m_dict;
};

static int
update_global_state_for_extension(PyThreadState *tstate,
PyObject *mod, PyModuleDef *def,
PyObject *name, PyObject *path)
PyObject *path, PyObject *name,
PyModuleDef *def,
struct singlephase_global_update *singlephase)
{
assert(mod != NULL && PyModule_Check(mod));
assert(def == _PyModule_GetDef(mod));

// bpo-44050: Extensions and def->m_base.m_copy can be updated
// when the extension module doesn't support sub-interpreters.
if (def->m_size == -1) {
if (!is_core_module(tstate->interp, name, path)) {
/* Copy the module's __dict__, if applicable. */
if (singlephase == NULL) {
assert(def->m_base.m_copy == NULL);
}
else {
assert(def->m_base.m_init != NULL
|| is_core_module(tstate->interp, name, path));
if (singlephase->m_dict == NULL) {
assert(def->m_base.m_copy == NULL);
}
else {
assert(PyDict_Check(singlephase->m_dict));
// gh-88216: Extensions and def->m_base.m_copy can be updated
// when the extension module doesn't support sub-interpreters.
assert(def->m_size == -1);
assert(!is_core_module(tstate->interp, name, path));
assert(PyUnicode_CompareWithASCIIString(name, "sys") != 0);
assert(PyUnicode_CompareWithASCIIString(name, "builtins") != 0);
if (def->m_base.m_copy) {
Expand All @@ -1206,17 +1238,16 @@ update_global_state_for_extension(PyThreadState *tstate,
XXX this should really not happen. */
Py_CLEAR(def->m_base.m_copy);
}
PyObject *dict = PyModule_GetDict(mod);
if (dict == NULL) {
return -1;
}
def->m_base.m_copy = PyDict_Copy(dict);
def->m_base.m_copy = PyDict_Copy(singlephase->m_dict);
if (def->m_base.m_copy == NULL) {
// XXX Ignore this error? Doing so would effectively
// mark the module as not loadable. */
return -1;
}
}
}

/* Add the module's def to the global cache. */
// XXX Why special-case the main interpreter?
if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) {
#ifndef NDEBUG
Expand Down Expand Up @@ -1258,6 +1289,8 @@ int
_PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
PyObject *filename, PyObject *modules)
{
PyThreadState *tstate = _PyThreadState_GET();

if (mod == NULL || !PyModule_Check(mod)) {
PyErr_BadInternalCall();
return -1;
Expand All @@ -1268,15 +1301,28 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
return -1;
}

PyThreadState *tstate = _PyThreadState_GET();
/* Only single-phase init extension modules can reach here. */
assert(is_singlephase(def));
assert(!is_core_module(tstate->interp, name, filename));
assert(!is_core_module(tstate->interp, name, name));

struct singlephase_global_update singlephase = {0};
// gh-88216: Extensions and def->m_base.m_copy can be updated
// when the extension module doesn't support sub-interpreters.
if (def->m_size == -1) {
singlephase.m_dict = PyModule_GetDict(mod);
assert(singlephase.m_dict != NULL);
}
if (update_global_state_for_extension(
tstate, mod, def, name, filename) < 0)
tstate, filename, name, def, &singlephase) < 0)
{
return -1;
}

if (finish_singlephase_extension(tstate, mod, def, name, modules) < 0) {
return -1;
}

return 0;
}

Expand Down Expand Up @@ -1407,11 +1453,25 @@ _PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name,
goto finally;
}

/* We only use _PyImport_FixupBuiltin() for the core builtin modules
* (sys and builtins). These modules are single-phase init with no
* module state, but we also don't populate def->m_base.m_copy
* for them. */
assert(is_core_module(tstate->interp, nameobj, nameobj));
assert(is_singlephase(def));
assert(def->m_size == -1);
assert(def->m_base.m_copy == NULL);

struct singlephase_global_update singlephase = {
/* We don't want def->m_base.m_copy populated. */
.m_dict=NULL,
};
if (update_global_state_for_extension(
tstate, mod, def, nameobj, nameobj) < 0)
tstate, nameobj, nameobj, def, &singlephase) < 0)
{
goto finally;
}

if (finish_singlephase_extension(tstate, mod, def, nameobj, modules) < 0) {
goto finally;
}
Expand Down Expand Up @@ -1444,6 +1504,7 @@ is_builtin(PyObject *name)
static PyObject*
create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec)
{
PyModuleDef *def = NULL;
PyObject *mod = import_find_extension(tstate, name, name);
if (mod || _PyErr_Occurred(tstate)) {
return mod;
Expand Down Expand Up @@ -1473,20 +1534,32 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec)
}

if (PyObject_TypeCheck(mod, &PyModuleDef_Type)) {
return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec);
def = (PyModuleDef*)mod;
assert(!is_singlephase(def));
return PyModule_FromDefAndSpec(def, spec);
}
else {
assert(PyModule_Check(mod));
PyModuleDef *def = PyModule_GetDef(mod);
def = PyModule_GetDef(mod);
if (def == NULL) {
return NULL;
}
assert(is_singlephase(def));

/* Remember pointer to module init function. */
def->m_base.m_init = p0;

struct singlephase_global_update singlephase = {0};
// gh-88216: Extensions and def->m_base.m_copy can be updated
// when the extension module doesn't support sub-interpreters.
if (def->m_size == -1
&& !is_core_module(tstate->interp, name, name))
{
singlephase.m_dict = PyModule_GetDict(mod);
assert(singlephase.m_dict != NULL);
}
if (update_global_state_for_extension(
tstate, mod, def, name, name) < 0)
tstate, name, name, def, &singlephase) < 0)
{
return NULL;
}
Expand Down