From 723c6fc97db6ae31235533ab458a2d9d151ee6c4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 18:23:02 -0700 Subject: [PATCH 1/6] Factor out _PyImport_RunDynamicModule(). --- Include/internal/pycore_importdl.h | 25 +++- Python/importdl.c | 222 +++++++++++++++++++---------- 2 files changed, 171 insertions(+), 76 deletions(-) diff --git a/Include/internal/pycore_importdl.h b/Include/internal/pycore_importdl.h index c8583582b358ac..9683152736eb70 100644 --- a/Include/internal/pycore_importdl.h +++ b/Include/internal/pycore_importdl.h @@ -14,10 +14,31 @@ extern "C" { extern const char *_PyImport_DynLoadFiletab[]; -extern PyObject *_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *); - typedef PyObject *(*PyModInitFunction)(void); +struct _Py_ext_module_loader_info { + PyObject *path; + PyObject *name; + PyObject *name_encoded; + const char *hook_prefix; +}; +extern void _Py_ext_module_loader_info_clear( + struct _Py_ext_module_loader_info *info); +extern int _Py_ext_module_loader_info_from_spec( + PyObject *spec, + struct _Py_ext_module_loader_info *info); + +struct _Py_ext_module_loader_result { + PyModuleDef *def; + PyObject *module; +}; +extern int _PyImport_RunDynamicModule( + struct _Py_ext_module_loader_info *info, + FILE *fp, + struct _Py_ext_module_loader_result *res); + +extern PyObject *_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *); + /* Max length of module suffix searched for -- accommodates "module.slb" */ #define MAXSUFFIXSIZE 12 diff --git a/Python/importdl.c b/Python/importdl.c index 7dfd301d77efb4..bb9b068e1cec1a 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -93,58 +93,91 @@ get_encoded_name(PyObject *name, const char **hook_prefix) { return NULL; } -PyObject * -_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) +void +_Py_ext_module_loader_info_clear(struct _Py_ext_module_loader_info *info) { -#ifndef MS_WINDOWS - PyObject *pathbytes = NULL; -#endif - PyObject *name_unicode = NULL, *name = NULL, *path = NULL, *m = NULL; - const char *name_buf, *hook_prefix; - const char *oldcontext, *newcontext; - dl_funcptr exportfunc; - PyModuleDef *def; - PyModInitFunction p0; + Py_CLEAR(info->path); + Py_CLEAR(info->name); + Py_CLEAR(info->name_encoded); +} + +int +_Py_ext_module_loader_info_from_spec(PyObject *spec, + struct _Py_ext_module_loader_info *info) +{ + PyObject *name_unicode = NULL, *name = NULL, *path = NULL; + const char *hook_prefix; name_unicode = PyObject_GetAttrString(spec, "name"); if (name_unicode == NULL) { - return NULL; + return -1; } if (!PyUnicode_Check(name_unicode)) { PyErr_SetString(PyExc_TypeError, "spec.name must be a string"); - goto error; - } - newcontext = PyUnicode_AsUTF8(name_unicode); - if (newcontext == NULL) { - goto error; + Py_DECREF(name_unicode); + return -1; } name = get_encoded_name(name_unicode, &hook_prefix); if (name == NULL) { - goto error; + Py_DECREF(name_unicode); + return -1; } - name_buf = PyBytes_AS_STRING(name); path = PyObject_GetAttrString(spec, "origin"); - if (path == NULL) - goto error; + if (path == NULL) { + Py_DECREF(name_unicode); + Py_DECREF(name); + return -1; + } + + *info = (struct _Py_ext_module_loader_info){ + .path=path, + .name=name_unicode, + .name_encoded=name, + .hook_prefix=hook_prefix, + }; + return 0; +} + +int +_PyImport_RunDynamicModule(struct _Py_ext_module_loader_info *info, + FILE *fp, + struct _Py_ext_module_loader_result *res) +{ +#ifndef MS_WINDOWS + PyObject *pathbytes = NULL; + const char *path_buf; +#endif + PyObject *m = NULL; + const char *name_buf = PyBytes_AS_STRING(info->name_encoded); + const char *oldcontext, *newcontext; + dl_funcptr exportfunc; + PyModInitFunction p0; + PyModuleDef *def; + + newcontext = PyUnicode_AsUTF8(info->name); + if (newcontext == NULL) { + return -1; + } - if (PySys_Audit("import", "OOOOO", name_unicode, path, + if (PySys_Audit("import", "OOOOO", info->name, info->path, Py_None, Py_None, Py_None) < 0) { - goto error; + return -1; } #ifdef MS_WINDOWS - exportfunc = _PyImport_FindSharedFuncptrWindows(hook_prefix, name_buf, - path, fp); + exportfunc = _PyImport_FindSharedFuncptrWindows( + info->hook_prefix, name_buf, info->path, fp); #else - pathbytes = PyUnicode_EncodeFSDefault(path); - if (pathbytes == NULL) - goto error; - exportfunc = _PyImport_FindSharedFuncptr(hook_prefix, name_buf, - PyBytes_AS_STRING(pathbytes), - fp); + pathbytes = PyUnicode_EncodeFSDefault(info->path); + if (pathbytes == NULL) { + return -1; + } + path_buf = PyBytes_AS_STRING(pathbytes); + exportfunc = _PyImport_FindSharedFuncptr( + info->hook_prefix, name_buf, path_buf, fp); Py_DECREF(pathbytes); #endif @@ -154,13 +187,13 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) msg = PyUnicode_FromFormat( "dynamic module does not define " "module export function (%s_%s)", - hook_prefix, name_buf); - if (msg == NULL) - goto error; - PyErr_SetImportError(msg, name_unicode, path); - Py_DECREF(msg); + info->hook_prefix, name_buf); + if (msg != NULL) { + PyErr_SetImportError(msg, info->name, info->path); + Py_DECREF(msg); + } } - goto error; + return -1; } p0 = (PyModInitFunction)exportfunc; @@ -177,14 +210,14 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) "initialization of %s failed without raising an exception", name_buf); } - goto error; + return -1; } else if (PyErr_Occurred()) { _PyErr_FormatFromCause( PyExc_SystemError, "initialization of %s raised unreported exception", name_buf); m = NULL; - goto error; + return -1; } if (Py_IS_TYPE(m, NULL)) { /* This can happen when a PyModuleDef is returned without calling @@ -194,60 +227,101 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) "init function of %s returned uninitialized object", name_buf); m = NULL; /* prevent segfault in DECREF */ - goto error; + return -1; } + if (PyObject_TypeCheck(m, &PyModuleDef_Type)) { - Py_DECREF(name_unicode); - Py_DECREF(name); - Py_DECREF(path); - return PyModule_FromDefAndSpec((PyModuleDef*)m, spec); + /* multi-phase init */ + + def = (PyModuleDef *)m; + m = NULL; + + /* Run PyModule_FromDefAndSpec() to finish loading the module. */ } + else { + /* single-phase init (legacy) */ - /* Fall back to single-phase init mechanism */ + /* Remember pointer to module init function. */ + def = PyModule_GetDef(m); + if (def == NULL) { + PyErr_Format(PyExc_SystemError, + "initialization of %s did not return an extension " + "module", name_buf); + Py_DECREF(m); + return -1; + } + def->m_base.m_init = p0; - if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) { - goto error; + if (info->hook_prefix == nonascii_prefix) { + /* don't allow legacy init for non-ASCII module names */ + PyErr_Format( + PyExc_SystemError, + "initialization of %s did not return PyModuleDef", + name_buf); + Py_DECREF(m); + return -1; + } + + /* Remember the filename as the __file__ attribute */ + if (PyModule_AddObjectRef(m, "__file__", info->path) < 0) { + PyErr_Clear(); /* Not important enough to report */ + } + + /* Run _PyImport_FixupExtensionObject() to finish loading the module. */ } - if (hook_prefix == nonascii_prefix) { - /* don't allow legacy init for non-ASCII module names */ - PyErr_Format( - PyExc_SystemError, - "initialization of %s did not return PyModuleDef", - name_buf); - goto error; + *res = (struct _Py_ext_module_loader_result){ + .def=def, + .module=m, + }; + return 0; +} + +PyObject * +_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) +{ + PyObject *m = NULL; + const char *name_buf; + //dl_funcptr exportfunc; + //PyModInitFunction p0; + PyModuleDef *def; + struct _Py_ext_module_loader_info info; + struct _Py_ext_module_loader_result res; + + if (_Py_ext_module_loader_info_from_spec(spec, &info) < 0) { + return NULL; } + name_buf = PyBytes_AS_STRING(info.name_encoded); - /* Remember pointer to module init function. */ - def = PyModule_GetDef(m); - if (def == NULL) { - PyErr_Format(PyExc_SystemError, - "initialization of %s did not return an extension " - "module", name_buf); - goto error; + if (_PyImport_RunDynamicModule(&info, fp, &res) < 0) { + _Py_ext_module_loader_info_clear(&info); + return NULL; } - def->m_base.m_init = p0; + m = res.module; + def = res.def; - /* Remember the filename as the __file__ attribute */ - if (PyModule_AddObjectRef(m, "__file__", path) < 0) { - PyErr_Clear(); /* Not important enough to report */ + if (m == NULL) { + _Py_ext_module_loader_info_clear(&info); + return PyModule_FromDefAndSpec(def, spec); } - PyObject *modules = PyImport_GetModuleDict(); - if (_PyImport_FixupExtensionObject(m, name_unicode, path, modules) < 0) + /* Fall back to single-phase init mechanism */ + + if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) { goto error; + } - Py_DECREF(name_unicode); - Py_DECREF(name); - Py_DECREF(path); + PyObject *modules = PyImport_GetModuleDict(); + if (_PyImport_FixupExtensionObject(m, info.name, info.path, modules) < 0) { + goto error; + } + _Py_ext_module_loader_info_clear(&info); return m; error: - Py_DECREF(name_unicode); - Py_XDECREF(name); - Py_XDECREF(path); - Py_XDECREF(m); + _Py_ext_module_loader_info_clear(&info); + Py_DECREF(m); return NULL; } From c5bf024cabe44876a53e280db8b176f9faf6f9f0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 18:44:38 -0700 Subject: [PATCH 2/6] Drop _PyImport_LoadDynamicModuleWithSpec(). --- Include/internal/pycore_importdl.h | 2 - Python/import.c | 100 ++++++++++++++++++----------- Python/importdl.c | 48 -------------- 3 files changed, 61 insertions(+), 89 deletions(-) diff --git a/Include/internal/pycore_importdl.h b/Include/internal/pycore_importdl.h index 9683152736eb70..ce9c6366b510ad 100644 --- a/Include/internal/pycore_importdl.h +++ b/Include/internal/pycore_importdl.h @@ -37,8 +37,6 @@ extern int _PyImport_RunDynamicModule( FILE *fp, struct _Py_ext_module_loader_result *res); -extern PyObject *_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *); - /* Max length of module suffix searched for -- accommodates "module.slb" */ #define MAXSUFFIXSIZE 12 diff --git a/Python/import.c b/Python/import.c index 6544a84d895d4a..f92bfeaf68b21d 100644 --- a/Python/import.c +++ b/Python/import.c @@ -619,9 +619,9 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) (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 - 4. _PyImport_LoadDynamicModuleWithSpec(): call + 2. _imp_create_dynamic_impl() -> _PyImport_RunDynamicModule() + 3. _PyImport_RunDynamicModule(): load + 4. _PyImport_RunDynamicModule(): call 5. -> PyModule_Create() -> PyModule_Create2() -> PyModule_CreateInitialized() 6. PyModule_CreateInitialized() -> PyModule_New() 7. PyModule_CreateInitialized(): allocate mod->md_state @@ -629,13 +629,13 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) 9. PyModule_CreateInitialized() -> PyModule_SetDocString() 10. PyModule_CreateInitialized(): set mod->md_def 11. : 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 + 12. _PyImport_RunDynamicModule(): set def->m_base.m_init + 13. _PyImport_RunDynamicModule(): set __file__ + 14. _imp_create_dynamic_impl() -> _PyImport_CheckSubinterpIncompatibleExtensionAllowed() + 15. _imp_create_dynamic_impl() -> _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 (6). subsequent times (found in _PyRuntime.imports.extensions): 1. _imp_create_dynamic_impl() -> import_find_extension() @@ -658,7 +658,7 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) (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 + 17. _PyImport_FixupExtensionObject(): add it to _PyRuntime.imports.extensions (6). previously loaded in main interpreter (found in _PyRuntime.imports.extensions): 1. _imp_create_dynamic_impl() -> import_find_extension() @@ -673,18 +673,18 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) (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: - 1. PyModule_FromDefAndSpec(): call its function - 8. 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() + 2. _imp_create_dynamic_impl() -> _PyImport_RunDynamicModule() + 3. _PyImport_RunDynamicModule(): load module init func + 4. _PyImport_RunDynamicModule(): call module init func + 5. _imp_create_dynamic_impl() -> PyModule_FromDefAndSpec() + 6. PyModule_FromDefAndSpec(): gather/check moduledef slots + 7. if there's a Py_mod_create slot: + 1. PyModule_FromDefAndSpec(): call its function + 8. 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() (10). every time: 1. _imp_exec_dynamic_impl() -> exec_builtin_or_dynamic() @@ -3717,44 +3717,66 @@ static PyObject * _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) /*[clinic end generated code: output=83249b827a4fde77 input=c31b954f4cf4e09d]*/ { - PyObject *mod, *name, *path; + PyObject *mod = NULL; FILE *fp; + struct _Py_ext_module_loader_info info; + struct _Py_ext_module_loader_result res; - name = PyObject_GetAttrString(spec, "name"); - if (name == NULL) { - return NULL; - } - - path = PyObject_GetAttrString(spec, "origin"); - if (path == NULL) { - Py_DECREF(name); + if (_Py_ext_module_loader_info_from_spec(spec, &info) < 0) { return NULL; } PyThreadState *tstate = _PyThreadState_GET(); - mod = import_find_extension(tstate, name, path); + mod = import_find_extension(tstate, info.name, info.path); if (mod != NULL || _PyErr_Occurred(tstate)) { assert(mod == NULL || !_PyErr_Occurred(tstate)); goto finally; } + /* Is multi-phase init or this is the first time being loaded. */ + if (file != NULL) { - fp = _Py_fopen_obj(path, "r"); + fp = _Py_fopen_obj(info.path, "r"); if (fp == NULL) { goto finally; } } - else + else { fp = NULL; + } + + if (_PyImport_RunDynamicModule(&info, fp, &res) < 0) { + goto finally; + } + mod = res.module; - mod = _PyImport_LoadDynamicModuleWithSpec(spec, fp); + if (mod == NULL) { + /* multi-phase init */ + + mod = PyModule_FromDefAndSpec(res.def, spec); + } + else { + /* Fall back to single-phase init mechanism */ + + const char *name_buf = PyBytes_AS_STRING(info.name_encoded); + if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) { + Py_CLEAR(mod); + goto finally; + } - if (fp) + PyObject *modules = PyImport_GetModuleDict(); + if (_PyImport_FixupExtensionObject(mod, info.name, info.path, modules) < 0) { + Py_CLEAR(mod); + goto finally; + } + } + + if (fp) { fclose(fp); + } finally: - Py_DECREF(name); - Py_DECREF(path); + _Py_ext_module_loader_info_clear(&info); return mod; } diff --git a/Python/importdl.c b/Python/importdl.c index bb9b068e1cec1a..bbfb4620806b7e 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -277,52 +277,4 @@ _PyImport_RunDynamicModule(struct _Py_ext_module_loader_info *info, return 0; } -PyObject * -_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) -{ - PyObject *m = NULL; - const char *name_buf; - //dl_funcptr exportfunc; - //PyModInitFunction p0; - PyModuleDef *def; - struct _Py_ext_module_loader_info info; - struct _Py_ext_module_loader_result res; - - if (_Py_ext_module_loader_info_from_spec(spec, &info) < 0) { - return NULL; - } - name_buf = PyBytes_AS_STRING(info.name_encoded); - - if (_PyImport_RunDynamicModule(&info, fp, &res) < 0) { - _Py_ext_module_loader_info_clear(&info); - return NULL; - } - m = res.module; - def = res.def; - - if (m == NULL) { - _Py_ext_module_loader_info_clear(&info); - return PyModule_FromDefAndSpec(def, spec); - } - - /* Fall back to single-phase init mechanism */ - - if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) { - goto error; - } - - PyObject *modules = PyImport_GetModuleDict(); - if (_PyImport_FixupExtensionObject(m, info.name, info.path, modules) < 0) { - goto error; - } - - _Py_ext_module_loader_info_clear(&info); - return m; - -error: - _Py_ext_module_loader_info_clear(&info); - Py_DECREF(m); - return NULL; -} - #endif /* HAVE_DYNAMIC_LOADING */ From e9dfa14605d63f51c6d09d56bf52804127c6b6f0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 19:10:34 -0700 Subject: [PATCH 3/6] Add an assert. --- Python/import.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/import.c b/Python/import.c index f92bfeaf68b21d..734f245f741398 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1202,6 +1202,7 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) // XXX Why special-case the main interpreter? if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { + assert(_extensions_cache_get(filename, name) == NULL); if (_extensions_cache_set(filename, name, def) < 0) { return -1; } From 3fa10354f506b408d987b75d3be8b48b97c5b0fb Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 19:43:21 -0700 Subject: [PATCH 4/6] Factor out fix_up_extension_for_interpreter(). --- Include/internal/pycore_import.h | 1 + Python/import.c | 114 ++++++++++++++++++++----------- 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h index eb8a9a0db46c22..cb6c957c09fa7e 100644 --- a/Include/internal/pycore_import.h +++ b/Include/internal/pycore_import.h @@ -27,6 +27,7 @@ extern int _PyImport_FixupBuiltin( 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 734f245f741398..2fc1946acfe6a1 100644 --- a/Python/import.c +++ b/Python/import.c @@ -200,16 +200,22 @@ _PyImport_ClearModules(PyInterpreterState *interp) Py_SETREF(MODULES(interp), NULL); } -PyObject * -PyImport_GetModuleDict(void) +static inline PyObject * +get_modules_dict(PyInterpreterState *interp) { - PyInterpreterState *interp = _PyInterpreterState_GET(); if (MODULES(interp) == NULL) { Py_FatalError("interpreter has no modules dictionary"); } return MODULES(interp); } +PyObject * +PyImport_GetModuleDict(void) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + return get_modules_dict(interp); +} + int _PyImport_SetModule(PyObject *name, PyObject *m) { @@ -632,10 +638,12 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) 12. _PyImport_RunDynamicModule(): set def->m_base.m_init 13. _PyImport_RunDynamicModule(): set __file__ 14. _imp_create_dynamic_impl() -> _PyImport_CheckSubinterpIncompatibleExtensionAllowed() - 15. _imp_create_dynamic_impl() -> _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 + 15. _imp_create_dynamic_impl() -> fix_up_extension() + 16. fix_up_extension() -> fix_up_extension_for_interpreter() + 17. fix_up_extension_for_interpreter(): set it on sys.modules + 18. fix_up_extension_for_interpreter(): add it to interp->imports.modules_by_index + 19. fix_up_extension_for_interpreter(): copy __dict__ into def->m_base.m_copy + 20. fix_up_extension(): add it to _PyRuntime.imports.extensions (6). subsequent times (found in _PyRuntime.imports.extensions): 1. _imp_create_dynamic_impl() -> import_find_extension() @@ -657,8 +665,8 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) 1-16. (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 + 1-19. (same as for m_size == -1) + 20. fix_up_extension(): add it to _PyRuntime.imports.extensions (6). previously loaded in main interpreter (found in _PyRuntime.imports.extensions): 1. _imp_create_dynamic_impl() -> import_find_extension() @@ -894,7 +902,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(). @@ -1159,28 +1167,29 @@ is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename) } static int -fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) +fix_up_extension_for_interpreter(PyInterpreterState *interp, + PyObject *mod, PyModuleDef *def, + PyObject *name, PyObject *filename, + PyObject *modules) { - if (mod == NULL || !PyModule_Check(mod)) { - PyErr_BadInternalCall(); - return -1; - } + assert(mod != NULL && PyModule_Check(mod)); + assert(def == PyModule_GetDef(mod)); - struct PyModuleDef *def = PyModule_GetDef(mod); - if (!def) { - PyErr_BadInternalCall(); + if (modules == NULL) { + modules = get_modules_dict(interp); + } + if (PyObject_SetItem(modules, name, mod) < 0) { return -1; } - PyThreadState *tstate = _PyThreadState_GET(); - if (_modules_by_index_set(tstate->interp, def, mod) < 0) { - return -1; + if (_modules_by_index_set(interp, def, mod) < 0) { + goto error; } // 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, filename)) { + if (!is_core_module(interp, name, filename)) { assert(PyUnicode_CompareWithASCIIString(name, "sys") != 0); assert(PyUnicode_CompareWithASCIIString(name, "builtins") != 0); if (def->m_base.m_copy) { @@ -1191,18 +1200,49 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) } PyObject *dict = PyModule_GetDict(mod); if (dict == NULL) { - return -1; + goto error; } def->m_base.m_copy = PyDict_Copy(dict); if (def->m_base.m_copy == NULL) { - return -1; + goto error; } } } + return 0; + +error: + PyMapping_DelItem(modules, name); + return -1; +} + + +static int +fix_up_extension(PyObject *mod, PyModuleDef *def, + PyObject *name, PyObject *filename, + PyObject *modules) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (def == NULL) { + def = PyModule_GetDef(mod); + if (def == NULL) { + PyErr_BadInternalCall(); + return -1; + } + } + + if (fix_up_extension_for_interpreter( + interp, mod, def, name, filename, modules) < 0) + { + return -1; + } + // XXX Why special-case the main interpreter? - if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { - assert(_extensions_cache_get(filename, name) == NULL); + if (_Py_IsMainInterpreter(interp) || def->m_size == -1) { +#ifndef NDEBUG + PyModuleDef *cached = _extensions_cache_get(filename, name); + assert(cached == NULL || cached == def); +#endif if (_extensions_cache_set(filename, name, def) < 0) { return -1; } @@ -1215,11 +1255,11 @@ 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); + if (fix_up_extension(mod, NULL, name, filename, modules) < 0) { return -1; } return 0; @@ -1342,11 +1382,8 @@ _PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules) if (nameobj == NULL) { return -1; } - if (PyObject_SetItem(modules, nameobj, mod) < 0) { - goto finally; - } - if (fix_up_extension(mod, nameobj, nameobj) < 0) { - PyMapping_DelItem(modules, nameobj); + assert(mod != NULL && PyModule_Check(mod)); + if (fix_up_extension(mod, NULL, nameobj, nameobj, modules) < 0) { goto finally; } res = 0; @@ -1398,15 +1435,15 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec) return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec); } else { + assert(PyModule_Check(mod)); /* Remember pointer to module init function. */ PyModuleDef *def = PyModule_GetDef(mod); if (def == NULL) { return NULL; } - def->m_base.m_init = p->initfunc; - if (_PyImport_FixupExtensionObject(mod, name, name, - modules) < 0) { + + if (fix_up_extension(mod, def, name, name, modules) < 0) { return NULL; } return mod; @@ -3765,8 +3802,7 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) goto finally; } - PyObject *modules = PyImport_GetModuleDict(); - if (_PyImport_FixupExtensionObject(mod, info.name, info.path, modules) < 0) { + if (fix_up_extension(mod, res.def, info.name, info.path, NULL) < 0) { Py_CLEAR(mod); goto finally; } From f103ccfbf1f2a61d464e64aadbd2d3c2030f8468 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 19:57:37 -0700 Subject: [PATCH 5/6] Do _extensions_cache_set() first. --- Python/import.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/Python/import.c b/Python/import.c index 2fc1946acfe6a1..1c5a36e1316191 100644 --- a/Python/import.c +++ b/Python/import.c @@ -639,11 +639,11 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) 13. _PyImport_RunDynamicModule(): set __file__ 14. _imp_create_dynamic_impl() -> _PyImport_CheckSubinterpIncompatibleExtensionAllowed() 15. _imp_create_dynamic_impl() -> fix_up_extension() - 16. fix_up_extension() -> fix_up_extension_for_interpreter() - 17. fix_up_extension_for_interpreter(): set it on sys.modules - 18. fix_up_extension_for_interpreter(): add it to interp->imports.modules_by_index - 19. fix_up_extension_for_interpreter(): copy __dict__ into def->m_base.m_copy - 20. fix_up_extension(): add it to _PyRuntime.imports.extensions + 16. fix_up_extension(): add it to _PyRuntime.imports.extensions + 17. fix_up_extension() -> fix_up_extension_for_interpreter() + 18. fix_up_extension_for_interpreter(): set it on sys.modules + 19. fix_up_extension_for_interpreter(): add it to interp->imports.modules_by_index + 20. fix_up_extension_for_interpreter(): copy __dict__ into def->m_base.m_copy (6). subsequent times (found in _PyRuntime.imports.extensions): 1. _imp_create_dynamic_impl() -> import_find_extension() @@ -662,11 +662,12 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) ...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) + 1-20. (same as for m_size == -1) (6). main interpreter - first time (not found in _PyRuntime.imports.extensions): - 1-19. (same as for m_size == -1) - 20. fix_up_extension(): add it to _PyRuntime.imports.extensions + 1-15. (same as for m_size == -1) + 16. fix_up_extension(): add it to _PyRuntime.imports.extensions + 17-20. (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() @@ -1231,12 +1232,6 @@ fix_up_extension(PyObject *mod, PyModuleDef *def, } } - if (fix_up_extension_for_interpreter( - interp, mod, def, name, filename, modules) < 0) - { - return -1; - } - // XXX Why special-case the main interpreter? if (_Py_IsMainInterpreter(interp) || def->m_size == -1) { #ifndef NDEBUG @@ -1248,6 +1243,12 @@ fix_up_extension(PyObject *mod, PyModuleDef *def, } } + if (fix_up_extension_for_interpreter( + interp, mod, def, name, filename, modules) < 0) + { + return -1; + } + return 0; } From 1405a59a9ede97b0d45d2cf54582727fce496052 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 7 Feb 2024 20:30:42 -0700 Subject: [PATCH 6/6] Load extension modules under the main interpreter first. --- Python/import.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/Python/import.c b/Python/import.c index 1c5a36e1316191..9e4bd4556a9a5d 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1132,6 +1132,48 @@ _PyImport_CheckSubinterpIncompatibleExtensionAllowed(const char *name) return 0; } +static PyThreadState * +maybe_switch_to_main_interpreter(PyThreadState *tstate) +{ + PyThreadState *main_tstate = tstate; + if (check_multi_interp_extensions(tstate->interp)) { + /* + If the module is single-phase init then the import + will fail. However, the module's init function will still + get run. That means it may still store state in the + shared-object/DLL address space (which never gets + closed/cleared), including objects (e.g. static types). + + This is a problem for isolated subinterpreters since each + has its own object allocator. If the loaded shared-object + still holds a reference to an object after the corresponding + interpreter has finalized then either we must let it leak + or else any later use of that object by another interpreter + (or across multiple init-fini cycles) will crash the process. + + We avoid the problem by first loading the module in the main + interpreter. + + Here's another complication: the module's init function might + register callbacks, whether in Python (e.g. sys.stdin, atexit) + or in linked libraries. Thus we cannot just dlclose() the + module in this error case. + */ + main_tstate = PyThreadState_New(_PyInterpreterState_Main()); + if (main_tstate == NULL) { + return NULL; + } + main_tstate->_whence = _PyThreadState_WHENCE_EXEC; +#ifndef NDEBUG + PyThreadState *old_tstate = PyThreadState_Swap(main_tstate); + assert(old_tstate == tstate); +#else + (void)PyThreadState_Swap(main_tstate); +#endif + } + return main_tstate; +} + static PyObject * get_core_module_dict(PyInterpreterState *interp, PyObject *name, PyObject *filename) @@ -3757,13 +3799,14 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) /*[clinic end generated code: output=83249b827a4fde77 input=c31b954f4cf4e09d]*/ { PyObject *mod = NULL; - FILE *fp; + FILE *fp = NULL; struct _Py_ext_module_loader_info info; struct _Py_ext_module_loader_result res; if (_Py_ext_module_loader_info_from_spec(spec, &info) < 0) { return NULL; } + const char *name_buf = PyBytes_AS_STRING(info.name_encoded); PyThreadState *tstate = _PyThreadState_GET(); mod = import_find_extension(tstate, info.name, info.path); @@ -3784,11 +3827,35 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) fp = NULL; } + PyThreadState *main_tstate = maybe_switch_to_main_interpreter(tstate); + if (_PyImport_RunDynamicModule(&info, fp, &res) < 0) { goto finally; } mod = res.module; + if (main_tstate != tstate) { + /* The init func ran under the main interpreter. + Now we must switch back and run it again under + the original interpreter. Only then should we + finish loading the module. */ + int is_singlephase = (mod != NULL); + Py_CLEAR(mod); + (void)PyThreadState_Swap(tstate); + if (is_singlephase) { + /* maybe_switch_to_main_interpreter() switched interpreters + only if it was an isolated interpreter. Thus we know + that a single-phase init module is incompatible. */ + (void)_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf); + assert(PyErr_Occurred()); + goto finally; + } + if (_PyImport_RunDynamicModule(&info, fp, &res) < 0) { + goto finally; + } + mod = res.module; + } + if (mod == NULL) { /* multi-phase init */ @@ -3809,11 +3876,11 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) } } - if (fp) { +finally: + if (fp != NULL) { fclose(fp); } -finally: _Py_ext_module_loader_info_clear(&info); return mod; }