https://github.com/python/cpython/commit/af3c1d817d3f8369f8003965d967332a3a721a25
commit: af3c1d817d3f8369f8003965d967332a3a721a25
branch: main
author: Eric Snow <[email protected]>
committer: ericsnowcurrently <[email protected]>
date: 2024-04-24T09:55:48-06:00
summary:
gh-117953: Cleanups For fix_up_extension() in import.c (gh-118192)
These are cleanups I've pulled out of gh-118116. Mostly, this change moves
code around to align with some future changes and to improve clarity a little.
There is one very small change in behavior: we now add the module to the
per-interpreter caches after updating the global state, rather than before.
files:
M Include/internal/pycore_import.h
M Python/import.c
M Python/importdl.c
M Python/pylifecycle.c
M Python/sysmodule.c
diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
index 08af53258cde97..8d7f0543f8d315 100644
--- a/Include/internal/pycore_import.h
+++ b/Include/internal/pycore_import.h
@@ -24,10 +24,12 @@ extern int _PyImport_ReleaseLock(PyInterpreterState
*interp);
// This is used exclusively for the sys and builtins modules:
extern int _PyImport_FixupBuiltin(
+ PyThreadState *tstate,
PyObject *mod,
const char *name, /* UTF-8 encoded string */
PyObject *modules
);
+// We could probably drop this:
extern int _PyImport_FixupExtensionObject(PyObject*, PyObject *,
PyObject *, PyObject *);
diff --git a/Python/import.c b/Python/import.c
index 8cdc04f03dd201..30d8082607ab37 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -200,39 +200,54 @@ _PyImport_ClearModules(PyInterpreterState *interp)
Py_SETREF(MODULES(interp), NULL);
}
+static inline PyObject *
+get_modules_dict(PyThreadState *tstate, bool fatal)
+{
+ /* Technically, it would make sense to incref the dict,
+ * since sys.modules could be swapped out and decref'ed to 0
+ * before the caller is done using it. However, that is highly
+ * unlikely, especially since we can rely on a global lock
+ * (i.e. the GIL) for thread-safety. */
+ PyObject *modules = MODULES(tstate->interp);
+ if (modules == NULL) {
+ if (fatal) {
+ Py_FatalError("interpreter has no modules dictionary");
+ }
+ _PyErr_SetString(tstate, PyExc_RuntimeError,
+ "unable to get sys.modules");
+ return NULL;
+ }
+ return modules;
+}
+
PyObject *
PyImport_GetModuleDict(void)
{
- PyInterpreterState *interp = _PyInterpreterState_GET();
- if (MODULES(interp) == NULL) {
- Py_FatalError("interpreter has no modules dictionary");
- }
- return MODULES(interp);
+ PyThreadState *tstate = _PyThreadState_GET();
+ return get_modules_dict(tstate, true);
}
int
_PyImport_SetModule(PyObject *name, PyObject *m)
{
- PyInterpreterState *interp = _PyInterpreterState_GET();
- PyObject *modules = MODULES(interp);
+ PyThreadState *tstate = _PyThreadState_GET();
+ PyObject *modules = get_modules_dict(tstate, true);
return PyObject_SetItem(modules, name, m);
}
int
_PyImport_SetModuleString(const char *name, PyObject *m)
{
- PyInterpreterState *interp = _PyInterpreterState_GET();
- PyObject *modules = MODULES(interp);
+ PyThreadState *tstate = _PyThreadState_GET();
+ PyObject *modules = get_modules_dict(tstate, true);
return PyMapping_SetItemString(modules, name, m);
}
static PyObject *
import_get_module(PyThreadState *tstate, PyObject *name)
{
- PyObject *modules = MODULES(tstate->interp);
+ PyObject *modules = get_modules_dict(tstate, false);
if (modules == NULL) {
- _PyErr_SetString(tstate, PyExc_RuntimeError,
- "unable to get sys.modules");
return NULL;
}
@@ -297,10 +312,8 @@ PyImport_GetModule(PyObject *name)
static PyObject *
import_add_module(PyThreadState *tstate, PyObject *name)
{
- PyObject *modules = MODULES(tstate->interp);
+ PyObject *modules = get_modules_dict(tstate, false);
if (modules == NULL) {
- _PyErr_SetString(tstate, PyExc_RuntimeError,
- "no import module dictionary");
return NULL;
}
@@ -397,7 +410,7 @@ remove_module(PyThreadState *tstate, PyObject *name)
{
PyObject *exc = _PyErr_GetRaisedException(tstate);
- PyObject *modules = MODULES(tstate->interp);
+ PyObject *modules = get_modules_dict(tstate, true);
if (PyDict_CheckExact(modules)) {
// Error is reported to the caller
(void)PyDict_Pop(modules, name, NULL);
@@ -618,77 +631,91 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
...for single-phase init modules, where m_size == -1:
(6). first time (not found in _PyRuntime.imports.extensions):
- 1. _imp_create_dynamic_impl() -> import_find_extension()
- 2. _imp_create_dynamic_impl() -> _PyImport_LoadDynamicModuleWithSpec()
- 3. _PyImport_LoadDynamicModuleWithSpec(): load <module init func>
- 4. _PyImport_LoadDynamicModuleWithSpec(): call <module init func>
- 5. <module init func> -> PyModule_Create() -> PyModule_Create2()
-> PyModule_CreateInitialized()
- 6. PyModule_CreateInitialized() -> PyModule_New()
- 7. PyModule_CreateInitialized(): allocate mod->md_state
- 8. PyModule_CreateInitialized() -> PyModule_AddFunctions()
- 9. PyModule_CreateInitialized() -> PyModule_SetDocString()
- 10. PyModule_CreateInitialized(): set mod->md_def
- 11. <module init func>: initialize the module
- 12. _PyImport_LoadDynamicModuleWithSpec() ->
_PyImport_CheckSubinterpIncompatibleExtensionAllowed()
- 13. _PyImport_LoadDynamicModuleWithSpec(): set def->m_base.m_init
- 14. _PyImport_LoadDynamicModuleWithSpec(): set __file__
- 15. _PyImport_LoadDynamicModuleWithSpec() ->
_PyImport_FixupExtensionObject()
- 16. _PyImport_FixupExtensionObject(): add it to
interp->imports.modules_by_index
- 17. _PyImport_FixupExtensionObject(): copy __dict__ into
def->m_base.m_copy
- 18. _PyImport_FixupExtensionObject(): add it to
_PyRuntime.imports.extensions
+ A. _imp_create_dynamic_impl() -> import_find_extension()
+ B. _imp_create_dynamic_impl() -> _PyImport_LoadDynamicModuleWithSpec()
+ C. _PyImport_LoadDynamicModuleWithSpec(): load <module init func>
+ D. _PyImport_LoadDynamicModuleWithSpec(): call <module init func>
+ E. <module init func> -> PyModule_Create() -> PyModule_Create2()
+ -> PyModule_CreateInitialized()
+ F. PyModule_CreateInitialized() -> PyModule_New()
+ G. PyModule_CreateInitialized(): allocate mod->md_state
+ H. PyModule_CreateInitialized() -> PyModule_AddFunctions()
+ I. PyModule_CreateInitialized() -> PyModule_SetDocString()
+ J. PyModule_CreateInitialized(): set mod->md_def
+ K. <module init func>: initialize the module, etc.
+ L. _PyImport_LoadDynamicModuleWithSpec()
+ -> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
+ M. _PyImport_LoadDynamicModuleWithSpec(): set def->m_base.m_init
+ N. _PyImport_LoadDynamicModuleWithSpec() ->
_PyImport_FixupExtensionObject()
+ O. _PyImport_FixupExtensionObject() ->
update_global_state_for_extension()
+ P. update_global_state_for_extension():
+ copy __dict__ into def->m_base.m_copy
+ Q. update_global_state_for_extension():
+ add it to _PyRuntime.imports.extensions
+ R. _PyImport_FixupExtensionObject() ->
finish_singlephase_extension()
+ S. finish_singlephase_extension():
+ add it to interp->imports.modules_by_index
+ T. finish_singlephase_extension(): add it to sys.modules
+ U. _imp_create_dynamic_impl(): set __file__
+
+ Step (P) is skipped for core modules (sys/builtins).
(6). subsequent times (found in _PyRuntime.imports.extensions):
- 1. _imp_create_dynamic_impl() -> import_find_extension()
- 2. import_find_extension() -> import_add_module()
- 3. if name in sys.modules: use that module
- 4. else:
+ A. _imp_create_dynamic_impl() -> import_find_extension()
+ B. import_find_extension() -> import_add_module()
+ C. if name in sys.modules: use that module
+ D. else:
1. import_add_module() -> PyModule_NewObject()
2. import_add_module(): set it on sys.modules
- 5. import_find_extension(): copy the "m_copy" dict into __dict__
- 6. _imp_create_dynamic_impl() ->
_PyImport_CheckSubinterpIncompatibleExtensionAllowed()
+ E. import_find_extension(): copy the "m_copy" dict into __dict__
+ F. _imp_create_dynamic_impl()
+ -> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
(10). (every time):
- 1. noop
+ A. noop
...for single-phase init modules, where m_size >= 0:
(6). not main interpreter and never loaded there - every time (not found
in _PyRuntime.imports.extensions):
- 1-16. (same as for m_size == -1)
+ A-N. (same as for m_size == -1)
+ O-Q. (skipped)
+ R-U. (same as for m_size == -1)
(6). main interpreter - first time (not found in
_PyRuntime.imports.extensions):
- 1-16. (same as for m_size == -1)
- 17. _PyImport_FixupExtensionObject(): add it to
_PyRuntime.imports.extensions
+ A-O. (same as for m_size == -1)
+ P. (skipped)
+ Q-U. (same as for m_size == -1)
(6). previously loaded in main interpreter (found in
_PyRuntime.imports.extensions):
- 1. _imp_create_dynamic_impl() -> import_find_extension()
- 2. import_find_extension(): call def->m_base.m_init
- 3. import_find_extension(): add the module to sys.modules
+ A. _imp_create_dynamic_impl() -> import_find_extension()
+ B. import_find_extension(): call def->m_base.m_init
+ C. import_find_extension(): add the module to sys.modules
(10). every time:
- 1. noop
+ A. noop
...for multi-phase init modules:
(6). every time:
- 1. _imp_create_dynamic_impl() -> import_find_extension() (not found)
- 2. _imp_create_dynamic_impl() -> _PyImport_LoadDynamicModuleWithSpec()
- 3. _PyImport_LoadDynamicModuleWithSpec(): load module init func
- 4. _PyImport_LoadDynamicModuleWithSpec(): call module init func
- 5. _PyImport_LoadDynamicModuleWithSpec() -> PyModule_FromDefAndSpec()
- 6. PyModule_FromDefAndSpec(): gather/check moduledef slots
- 7. if there's a Py_mod_create slot:
+ A. _imp_create_dynamic_impl() -> import_find_extension() (not found)
+ B. _imp_create_dynamic_impl() -> _PyImport_LoadDynamicModuleWithSpec()
+ C. _PyImport_LoadDynamicModuleWithSpec(): load module init func
+ D. _PyImport_LoadDynamicModuleWithSpec(): call module init func
+ E. _PyImport_LoadDynamicModuleWithSpec() -> PyModule_FromDefAndSpec()
+ F. PyModule_FromDefAndSpec(): gather/check moduledef slots
+ G. if there's a Py_mod_create slot:
1. PyModule_FromDefAndSpec(): call its function
- 8. else:
+ H. else:
1. PyModule_FromDefAndSpec() -> PyModule_NewObject()
- 9: PyModule_FromDefAndSpec(): set mod->md_def
- 10. PyModule_FromDefAndSpec() -> _add_methods_to_object()
- 11. PyModule_FromDefAndSpec() -> PyModule_SetDocString()
+ I: PyModule_FromDefAndSpec(): set mod->md_def
+ J. PyModule_FromDefAndSpec() -> _add_methods_to_object()
+ K. PyModule_FromDefAndSpec() -> PyModule_SetDocString()
(10). every time:
- 1. _imp_exec_dynamic_impl() -> exec_builtin_or_dynamic()
- 2. if mod->md_state == NULL (including if m_size == 0):
+ A. _imp_exec_dynamic_impl() -> exec_builtin_or_dynamic()
+ B. if mod->md_state == NULL (including if m_size == 0):
1. exec_builtin_or_dynamic() -> PyModule_ExecDef()
2. PyModule_ExecDef(): allocate mod->md_state
3. if there's a Py_mod_exec slot:
@@ -894,7 +921,7 @@ extensions_lock_release(void)
(module name, module name) (for built-in modules) or by
(filename, module name) (for dynamically loaded modules), containing these
modules. A copy of the module's dictionary is stored by calling
- _PyImport_FixupExtensionObject() immediately after the module initialization
+ fix_up_extension() immediately after the module initialization
function succeeds. A copy can be retrieved from there by calling
import_find_extension().
@@ -1158,24 +1185,14 @@ is_core_module(PyInterpreterState *interp, PyObject
*name, PyObject *path)
return 0;
}
+
static int
-fix_up_extension(PyObject *mod, PyObject *name, PyObject *path)
+update_global_state_for_extension(PyThreadState *tstate,
+ PyObject *mod, PyModuleDef *def,
+ PyObject *name, PyObject *path)
{
- if (mod == NULL || !PyModule_Check(mod)) {
- PyErr_BadInternalCall();
- return -1;
- }
-
- struct PyModuleDef *def = PyModule_GetDef(mod);
- if (!def) {
- PyErr_BadInternalCall();
- return -1;
- }
-
- PyThreadState *tstate = _PyThreadState_GET();
- if (_modules_by_index_set(tstate->interp, def, mod) < 0) {
- return -1;
- }
+ 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.
@@ -1202,6 +1219,10 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject
*path)
// XXX Why special-case the main interpreter?
if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) {
+#ifndef NDEBUG
+ PyModuleDef *cached = _extensions_cache_get(path, name);
+ assert(cached == NULL || cached == def);
+#endif
if (_extensions_cache_set(path, name, def) < 0) {
return -1;
}
@@ -1210,15 +1231,50 @@ fix_up_extension(PyObject *mod, PyObject *name,
PyObject *path)
return 0;
}
+/* For multi-phase init modules, the module is finished
+ * by PyModule_FromDefAndSpec(). */
+static int
+finish_singlephase_extension(PyThreadState *tstate,
+ PyObject *mod, PyModuleDef *def,
+ PyObject *name, PyObject *modules)
+{
+ assert(mod != NULL && PyModule_Check(mod));
+ assert(def == PyModule_GetDef(mod));
+
+ if (_modules_by_index_set(tstate->interp, def, mod) < 0) {
+ return -1;
+ }
+
+ if (modules != NULL) {
+ if (PyObject_SetItem(modules, name, mod) < 0) {
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
int
_PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
PyObject *filename, PyObject *modules)
{
- if (PyObject_SetItem(modules, name, mod) < 0) {
+ if (mod == NULL || !PyModule_Check(mod)) {
+ PyErr_BadInternalCall();
return -1;
}
- if (fix_up_extension(mod, name, filename) < 0) {
- PyMapping_DelItem(modules, name);
+ PyModuleDef *def = PyModule_GetDef(mod);
+ if (def == NULL) {
+ PyErr_BadInternalCall();
+ return -1;
+ }
+
+ PyThreadState *tstate = _PyThreadState_GET();
+ if (update_global_state_for_extension(
+ tstate, mod, def, name, filename) < 0)
+ {
+ return -1;
+ }
+ if (finish_singlephase_extension(tstate, mod, def, name, modules) < 0) {
return -1;
}
return 0;
@@ -1245,7 +1301,7 @@ import_find_extension(PyThreadState *tstate, PyObject
*name,
}
PyObject *mod, *mdict;
- PyObject *modules = MODULES(tstate->interp);
+ PyObject *modules = get_modules_dict(tstate, true);
if (def->m_size == -1) {
PyObject *m_copy = def->m_base.m_copy;
@@ -1333,7 +1389,8 @@ clear_singlephase_extension(PyInterpreterState *interp,
/*******************/
int
-_PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules)
+_PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name,
+ PyObject *modules)
{
int res = -1;
assert(mod != NULL && PyModule_Check(mod));
@@ -1350,11 +1407,12 @@ _PyImport_FixupBuiltin(PyObject *mod, const char *name,
PyObject *modules)
goto finally;
}
- if (PyObject_SetItem(modules, nameobj, mod) < 0) {
+ if (update_global_state_for_extension(
+ tstate, mod, def, nameobj, nameobj) < 0)
+ {
goto finally;
}
- if (fix_up_extension(mod, nameobj, nameobj) < 0) {
- PyMapping_DelItem(modules, nameobj);
+ if (finish_singlephase_extension(tstate, mod, def, nameobj, modules) < 0) {
goto finally;
}
@@ -1391,7 +1449,6 @@ create_builtin(PyThreadState *tstate, PyObject *name,
PyObject *spec)
return mod;
}
- PyObject *modules = MODULES(tstate->interp);
struct _inittab *found = NULL;
for (struct _inittab *p = INITTAB; p->name != NULL; p++) {
if (_PyUnicode_EqualToASCIIString(name, p->name)) {
@@ -1419,14 +1476,22 @@ create_builtin(PyThreadState *tstate, PyObject *name,
PyObject *spec)
return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec);
}
else {
- /* Remember pointer to module init function. */
+ assert(PyModule_Check(mod));
PyModuleDef *def = PyModule_GetDef(mod);
if (def == NULL) {
return NULL;
}
+ /* Remember pointer to module init function. */
def->m_base.m_init = p0;
- if (_PyImport_FixupExtensionObject(mod, name, name, modules) < 0) {
+
+ if (update_global_state_for_extension(
+ tstate, mod, def, name, name) < 0)
+ {
+ return NULL;
+ }
+ PyObject *modules = get_modules_dict(tstate, true);
+ if (finish_singlephase_extension(tstate, mod, def, name, modules) < 0)
{
return NULL;
}
return mod;
@@ -3783,12 +3848,6 @@ _imp_create_dynamic_impl(PyObject *module, PyObject
*spec, PyObject *file)
}
mod = _PyImport_LoadDynamicModuleWithSpec(spec, fp);
- if (mod != NULL) {
- /* Remember the filename as the __file__ attribute */
- if (PyModule_AddObjectRef(mod, "__file__", filename) < 0) {
- PyErr_Clear(); /* Not important enough to report */
- }
- }
// XXX Shouldn't this happen in the error cases too.
if (fp) {
diff --git a/Python/importdl.c b/Python/importdl.c
index 7cf30bea3a861a..e512161d3071f2 100644
--- a/Python/importdl.c
+++ b/Python/importdl.c
@@ -226,6 +226,11 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE
*fp)
}
def->m_base.m_init = p0;
+ /* Remember the filename as the __file__ attribute */
+ if (PyModule_AddObjectRef(m, "__file__", filename) < 0) {
+ PyErr_Clear(); /* Not important enough to report */
+ }
+
PyObject *modules = PyImport_GetModuleDict();
if (_PyImport_FixupExtensionObject(m, name_unicode, filename, modules) < 0)
goto error;
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index cc1824634e7a7f..0f3ca4a6687514 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -777,7 +777,7 @@ pycore_init_builtins(PyThreadState *tstate)
}
PyObject *modules = _PyImport_GetModules(interp);
- if (_PyImport_FixupBuiltin(bimod, "builtins", modules) < 0) {
+ if (_PyImport_FixupBuiltin(tstate, bimod, "builtins", modules) < 0) {
goto error;
}
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index 05ee4051a20e18..7af363678e8e86 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -3764,7 +3764,7 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p)
return status;
}
- if (_PyImport_FixupBuiltin(sysmod, "sys", modules) < 0) {
+ if (_PyImport_FixupBuiltin(tstate, sysmod, "sys", modules) < 0) {
goto error;
}
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
