8000 gh-117953: Split Up _PyImport_LoadDynamicModuleWithSpec() (gh-118203) · python/cpython@44f57a9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 44f57a9

Browse files
ericsnowcurrentlyericsnowcurrently
authored
gh-117953: Split Up _PyImport_LoadDynamicModuleWithSpec() (gh-118203)
Basically, I've turned most of _PyImport_LoadDynamicModuleWithSpec() into two new functions (_PyImport_GetModInitFunc() and _PyImport_RunModInitFunc()) and moved the rest of it out into _imp_create_dynamic_impl(). There shouldn't be any changes in behavior. This change makes some future changes simpler. This is particularly relevant to potentially calling each module init function in the main interpreter first. Thus the critical part of the PR is the addition of _PyImport_RunModInitFunc(), which is strictly focused on running the init func and validating the result. A later PR will take it a step farther by capturing error information rather than raising exceptions. FWIW, this change also helps readers by clarifying a bit more about what happens when an extension/builtin module is imported.
1 parent 23d0371 commit 44f57a9

File tree

5 files changed

+192
-144
lines changed

5 files changed

+192
-144
lines changed

‎Include/internal/pycore_import.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ extern int _PyImport_FixupBuiltin(
2929
const char *name, /* UTF-8 encoded string */
3030
PyObject *modules
3131
);
32-
// We could probably drop this:
33-
extern int _PyImport_FixupExtensionObject(PyObject*, PyObject *,
34-
PyObject *, PyObject *);
3532

3633
// Export for many shared extensions, like '_json'
3734
PyAPI_FUNC(PyObject*) _PyImport_GetModuleAttr(PyObject *, PyObject *);
@@ -55,7 +52,7 @@ struct _import_runtime_state {
5552
Only legacy (single-phase init) extension modules are added
5653
and only if they support multiple initialization (m_size >- 0)
5754
or are imported in the main interpreter.
58-
This is initialized lazily in _PyImport_FixupExtensionObject().
55+
This is initialized lazily in fix_up_extension() in import.c.
5956
Modules are added there and looked up in _imp.find_extension(). */
6057
_Py_hashtable_t *hashtable;
6158
} extensions;

‎Include/internal/pycore_importdl.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,17 @@ extern int _Py_ext_module_loader_info_init_from_spec(
4040
struct _Py_ext_module_loader_info *info,
4141
PyObject *spec);
4242

43-
extern PyObject *_PyImport_LoadDynamicModuleWithSpec(
43+
struct _Py_ext_module_loader_result {
44+
PyModuleDef *def;
45+
PyObject *module;
46+
};
47+
extern PyModInitFunction _PyImport_GetModInitFunc(
4448
struct _Py_ext_module_loader_info *info,
45-
PyObject *spec,
4649
FILE *fp);
50+
extern int _PyImport_RunModInitFunc(
51+
PyModInitFunction p0,
52+
struct _Py_ext_module_loader_info *info,
53+
struct _Py_ext_module_loader_result *p_res);
4754

4855

4956
/* Max length of module suffix searched for -- accommodates "module.slb" */

‎Include/moduleobject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ typedef struct PyModuleDef_Base {
5353
/* A copy of the module's __dict__ after the first time it was loaded.
5454
This is only set/used for legacy modules that do not support
5555
multiple initializations.
56-
It is set by _PyImport_FixupExtensionObject(). */
56+
It is set by fix_up_extension() in import.c. */
5757
PyObject* m_copy;
5858
} PyModuleDef_Base;
5959

‎Python/import.c

Lines changed: 126 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -632,44 +632,45 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
632632
633633
(6). first time (not found in _PyRuntime.imports.extensions):
634634
A. _imp_create_dynamic_impl() -> import_find_extension()
635-
B. _imp_create_dynamic_impl() -> _PyImport_LoadDynamicModuleWithSpec()
636-
C. _PyImport_LoadDynamicModuleWithSpec(): load <module init func>
637-
D. _PyImport_LoadDynamicModuleWithSpec(): call <module init func>
638-
E. <module init func> -> PyModule_Create() -> PyModule_Create2()
635+
B. _imp_create_dynamic_impl() -> _PyImport_GetModInitFunc()
636+
C. _PyImport_GetModInitFunc(): load <module init func>
637+
D. _imp_create_dynamic_impl() -> _PyImport_RunModInitFunc()
638+
E. _PyImport_RunModInitFunc(): call <module init func>
639+
F. <module init func> -> PyModule_Create() -> PyModule_Create2()
639640
-> PyModule_CreateInitialized()
640-
F. PyModule_CreateInitialized() -> PyModule_New()
641-
G. PyModule_CreateInitialized(): allocate mod->md_state
642-
H. PyModule_CreateInitialized() -> PyModule_AddFunctions()
643-
I. PyModule_CreateInitialized() -> PyModule_SetDocString()
644-
J. PyModule_CreateInitialized(): set mod->md_def
645-
K. <module init func>: initialize the module, etc.
646-
L. _PyImport_LoadDynamicModuleWithSpec()
647-
-> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
648-
M. _PyImport_LoadDynamicModuleWithSpec(): set def->m_base.m_init
649-
N. _PyImport_LoadDynamicModuleWithSpec() -> _PyImport_FixupExtensionObject()
650-
O. _PyImport_FixupExtensionObject() -> update_global_state_for_extension()
651-
P. update_global_state_for_extension():
652-
copy __dict__ into def->m_base.m_copy
653-
Q. update_global_state_for_extension():
654-
add it to _PyRuntime.imports.extensions
655-
R. _PyImport_FixupExtensionObject() -> finish_singlephase_extension()
656-
S. finish_singlephase_extension():
657-
add it to interp->imports.modules_by_index
658-
T. finish_singlephase_extension(): add it to sys.modules
659-
U. _imp_create_dynamic_impl(): set __file__
660-
661-
Step (P) is skipped for core modules (sys/builtins).
641+
G. PyModule_CreateInitialized() -> PyModule_New()
642+
H. PyModule_CreateInitialized(): allocate mod->md_state
643+
I. PyModule_CreateInitialized() -> PyModule_AddFunctions()
644+
J. PyModule_CreateInitialized() -> PyModule_SetDocString()
645+
K. PyModule_CreateInitialized(): set mod->md_def
646+
L. <module init func>: initialize the module, etc.
647+
M. _PyImport_RunModInitFunc(): set def->m_base.m_init
648+
N. _imp_create_dynamic_impl()
649+
-> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
650+
O. _imp_create_dynamic_impl(): set __file__
651+
P. _imp_create_dynamic_impl() -> update_global_state_for_extension()
652+
Q. update_global_state_for_extension():
653+
copy __dict__ into def->m_base.m_copy
654+
R. update_global_state_for_extension():
655+
add it to _PyRuntime.imports.extensions
656+
S. _imp_create_dynamic_impl() -> finish_singlephase_extension()
657+
T. finish_singlephase_extension():
658+
add it to interp->imports.modules_by_index
659+
U. finish_singlephase_extension(): add it to sys.modules
660+
661+
Step (Q) is skipped for core modules (sys/builtins).
662662
663663
(6). subsequent times (found in _PyRuntime.imports.extensions):
664664
A. _imp_create_dynamic_impl() -> import_find_extension()
665-
B. import_find_extension() -> import_add_module()
666-
C. if name in sys.modules: use that module
667-
D. else:
665+
B. import_find_extension()
666+
-> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
667+
C. import_find_extension() -> import_add_module()
668+
D. if name in sys.modules: use that module
669+
E. else:
668670
1. import_add_module() -> PyModule_NewObject()
669671
2. import_add_module(): set it on sys.modules
670-
E. import_find_extension(): copy the "m_copy" dict into __dict__
671-
F. _imp_create_dynamic_impl()
672-
-> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
672+
F. import_find_extension(): copy the "m_copy" dict into __dict__
673+
G. import_find_extension(): add to modules_by_index
673674
674675
(10). (every time):
675676
A. noop
@@ -678,19 +679,22 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
678679
...for single-phase init modules, where m_size >= 0:
679680
680681
(6). not main interpreter and never loaded there - every time (not found in _PyRuntime.imports.extensions):
681-
A-N. (same as for m_size == -1)
682-
O-Q. (skipped)
683-
R-U. (same as for m_size == -1)
682+
A-O. (same as for m_size == -1)
683+
P-R. (skipped)
684+
S-U. (same as for m_size == -1)
684685
685686
(6). main interpreter - first time (not found in _PyRuntime.imports.extensions):
686-
A-O. (same as for m_size == -1)
687-
P. (skipped)
688-
Q-U. (same as for m_size == -1)
687+
A-Q. (same as for m_size == -1)
688+
R. (skipped)
689+
S-U. (same as for m_size == -1)
689690
690-
(6). previously loaded in main interpreter (found in _PyRuntime.imports.extensions):
691+
(6). subsequent times (found in _PyRuntime.imports.extensions):
691692
A. _imp_create_dynamic_impl() -> import_find_extension()
692-
B. import_find_extension(): call def->m_base.m_init
693-
C. import_find_extension(): add the module to sys.modules
693+
B. import_find_extension()
694+
-> _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
695+
C. import_find_extension(): call def->m_base.m_init (see above)
696+
D. import_find_extension(): add the module to sys.modules
697+
E. import_find_extension(): add to modules_by_index
694698
695699
(10). every time:
696700
A. noop
@@ -1270,7 +1274,7 @@ finish_singlephase_extension(PyThreadState *tstate,
12701274
PyObject *name, PyObject *modules)
12711275
{
12721276
assert(mod != NULL && PyModule_Check(mod));
1273-
assert(def == PyModule_GetDef(mod));
1277+
assert(def == _PyModule_GetDef(mod));
12741278

12751279
if (_modules_by_index_set(tstate->interp, def, mod) < 0) {
12761280
return -1;
@@ -1285,47 +1289,6 @@ finish_singlephase_extension(PyThreadState *tstate,
12851289
return 0;
12861290
}
12871291

1288-
int
1289-
_PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
1290-
PyObject *filename, PyObject *modules)
1291-
{
1292-
PyThreadState *tstate = _PyThreadState_GET();
1293-
1294-
if (mod == NULL || !PyModule_Check(mod)) {
1295-
PyErr_BadInternalCall();
1296-
return -1;
1297-
}
1298-
PyModuleDef *def = PyModule_GetDef(mod);
1299-
if (def == NULL) {
1300-
PyErr_BadInternalCall();
1301-
return -1;
1302-
}
1303-
1304-
/* Only single-phase init extension modules can reach here. */
1305-
assert(is_singlephase(def));
1306-
assert(!is_core_module(tstate->interp, name, filename));
1307-
assert(!is_core_module(tstate->interp, name, name));
1308-
1309-
struct singlephase_global_update singlephase = {0};
1310-
// gh-88216: Extensions and def->m_base.m_copy can be updated
1311-
// when the extension module doesn't support sub-interpreters.
1312-
if (def->m_size == -1) {
1313-
singlephase.m_dict = PyModule_GetDict(mod);
1314-
assert(singlephase.m_dict != NULL);
1315-
}
1316-
if (update_global_state_for_extension(
1317-
tstate, filename, name, def, &singlephase) < 0)
1318-
{
1319-
return -1;
1320-
}
1321-
1322-
if (finish_singlephase_extension(tstate, mod, def, name, modules) < 0) {
1323-
return -1;
1324-
}
1325-
1326-
return 0;
1327-
}
1328-
13291292

13301293
static PyObject *
13311294
import_find_extension(PyThreadState *tstate,
@@ -1514,7 +1477,12 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec)
15141477
}
15151478

15161479
PyObject *mod = import_find_extension(tstate, &info);
1517-
if (mod || _PyErr_Occurred(tstate)) {
1480+
if (mod != NULL) {
1481+
assert(!_PyErr_Occurred(tstate));
1482+
assert(is_singlephase(_PyModule_GetDef(mod)));
1483+
goto finally;
1484+
}
1485+
else if (_PyErr_Occurred(tstate)) {
15181486
goto finally;
15191487
}
15201488

@@ -3900,31 +3868,35 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
39003868
/*[clinic end generated code: output=83249b827a4fde77 input=c31b954f4cf4e09d]*/
39013869
{
39023870
PyObject *mod = NULL;
3903-
FILE *fp;
3871+
PyModuleDef *def = NULL;
3872+
PyThreadState *tstate = _PyThreadState_GET();
39043873

39053874
struct _Py_ext_module_loader_info info;
39063875
if (_Py_ext_module_loader_info_init_from_spec(&info, spec) < 0) {
39073876
return NULL;
39083877
}
39093878

3910-
PyThreadState *tstate = _PyThreadState_GET();
39113879
mod = import_find_extension(tstate, &info);
3912-
if (mod != NULL || _PyErr_Occurred(tstate)) {
3913-
assert(mod == NULL || !_PyErr_Occurred(tstate));
3880+
if (mod != NULL) {
3881+
assert(!_PyErr_Occurred(tstate));
3882+
assert(is_singlephase(_PyModule_GetDef(mod)));
39143883
goto finally;
39153884
}
3885+
else if (_PyErr_Occurred(tstate)) {
3886+
goto finally;
3887+
}
3888+
/* Otherwise it must be multi-phase init or the first time it's loaded. */
39163889

39173890
if (PySys_Audit("import", "OOOOO", info.name, info.filename,
39183891
Py_None, Py_None, Py_None) < 0)
39193892
{
39203893
goto finally;
39213894
}
39223895

3923-
/* Is multi-phase init or this is the first time being loaded. */
3924-
39253896
/* We would move this (and the fclose() below) into
39263897
* _PyImport_GetModInitFunc(), but it isn't clear if the intervening
39273898
* code relies on fp still being open. */
3899+
FILE *fp;
39283900
if (file != NULL) {
39293901
fp = _Py_fopen_obj(info.filename, "r");
39303902
if (fp == NULL) {
@@ -3935,7 +3907,70 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
39353907
fp = NULL;
39363908
}
39373909

3938-
mod = _PyImport_LoadDynamicModuleWithSpec(&info, spec, fp);
3910+
PyModInitFunction p0 = _PyImport_GetModInitFunc(&info, fp);
3911+
if (p0 == NULL) {
3912+
goto finally;
3913+
}
3914+
3915+
struct _Py_ext_module_loader_result res;
3916+
if (_PyImport_RunModInitFunc(p0, &info, &res) < 0) {
3917+
assert(PyErr_Occurred());
3918+
goto finally;
3919+
}
3920+
3921+
mod = res.module;
3922+
res.module = NULL;
3923+
def = res.def;
3924+
assert(def != NULL);
3925+
3926+
if (mod == NULL) {
3927+
//assert(!is_singlephase(def));
3928+
mod = PyModule_FromDefAndSpec(def, spec);
3929+
if (mod == NULL) {
3930+
goto finally;
3931+
}
3932+
}
3933+
else {
3934+
assert(is_singlephase(def));
3935+
assert(!is_core_module(tstate->interp, info.name, info.filename));
3936+
assert(!is_core_module(tstate->interp, info.name, info.name));
3937+
3938+
const char *name_buf = PyBytes_AS_STRING(info.name_encoded);
3939+
if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) {
3940+
Py_CLEAR(mod);
3941+
goto finally;
3942+
}
3943+
3944+
/* Remember pointer to module init function. */
3945+
res.def->m_base.m_init = p0;
3946+
3947+
/* Remember the filename as the __file__ attribute */
3948+
if (PyModule_AddObjectRef(mod, "__file__", info.filename) < 0) {
3949+
PyErr_Clear(); /* Not important enough to report */
3950+
}
3951+
3952+
struct singlephase_global_update singlephase = {0};
3953+
// gh-88216: Extensions and def->m_base.m_copy can be updated
3954+
// when the extension module doesn't support sub-interpreters.
3955+
if (def->m_size == -1) {
3956+
singlephase.m_dict = PyModule_GetDict(mod);
3957+
assert(singlephase.m_dict != NULL);
3958+
}
3959+
if (update_global_state_for_extension(
3960+
tstate, info.filename, info.name, def, &singlephase) < 0)
3961+
{
3962+
Py_CLEAR(mod);
3963+
goto finally;
3964+
}
3965+
3966+
PyObject *modules = get_modules_dict(tstate, true);
3967+
if (finish_singlephase_extension(
3968+
tstate, mod, def, info.name, modules) < 0)
3969+
{
3970+
Py_CLEAR(mod);
3971+
goto finally;
3972+
}
3973+
}
39393974

39403975
// XXX Shouldn't this happen in the error cases too.
39413976
if (fp) {

0 commit comments

Comments
 (0)
0