Skip to content

Commit

Permalink
gh-102660: Fix is_core_module() (gh-103257)
Browse files Browse the repository at this point in the history
In gh-102744 we added is_core_module() (in Python/import.c), which relies on get_core_module_dict() (also added in that PR).  The problem is that_PyImport_FixupBuiltin(), which ultimately calls is_core_module(), is called on the builtins module before interp->builtins_copyis set.  Consequently, the builtins module isn't considered a "core" module while it is getting "fixed up" and its module def m_copy erroneously gets set.  Under isolated interpreters this causes problems since sys and builtins are allowed even though they are still single-phase init modules.  (This was discovered while working on gh-101660.)

The solution is to stop relying on get_core_module_dict() in is_core_module().
  • Loading branch information
ericsnowcurrently authored Apr 4, 2023
1 parent bceb9e0 commit f513d5c
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,17 @@ get_core_module_dict(PyInterpreterState *interp,
static inline int
is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename)
{
return get_core_module_dict(interp, name, filename) != NULL;
/* This might be called before the core dict copies are in place,
so we can't rely on get_core_module_dict() here. */
if (filename == name) {
if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) {
return 1;
}
if (PyUnicode_CompareWithASCIIString(name, "builtins") == 0) {
return 1;
}
}
return 0;
}

static int
Expand All @@ -1136,6 +1146,8 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)
// when the extension module doesn't support sub-interpreters.
if (def->m_size == -1) {
if (!is_core_module(tstate->interp, name, filename)) {
assert(PyUnicode_CompareWithASCIIString(name, "sys") != 0);
assert(PyUnicode_CompareWithASCIIString(name, "builtins") != 0);
if (def->m_base.m_copy) {
/* Somebody already imported the module,
likely under a different name.
Expand Down

0 comments on commit f513d5c

Please sign in to comment.