Skip to content

Commit d393c1b

Browse files
ericsnowcurrentlyericsnowcurrently
authored
bpo-28411: Isolate PyInterpreterState.modules (#3575)
A bunch of code currently uses PyInterpreterState.modules directly instead of PyImport_GetModuleDict(). This complicates efforts to make changes relative to sys.modules. This patch switches to using PyImport_GetModuleDict() uniformly. Also, a number of related uses of sys.modules are updated for uniformity for the same reason. Note that this code was already reviewed and merged as part of #1638. I reverted that and am now splitting it up into more focused parts.
1 parent 8dcf22f commit d393c1b

File tree

10 files changed

+116
-55
lines changed

10 files changed

+116
-55
lines changed

‎Include/import.h

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,17 @@ PyAPI_FUNC(PyObject *) PyImport_ExecCodeModuleObject(
3838
);
3939
#endif
4040
PyAPI_FUNC(PyObject *) PyImport_GetModuleDict(void);
41+
#ifndef Py_LIMITED_API
42+
PyAPI_FUNC(int) _PyImport_IsInitialized(PyInterpreterState *);
43+
#endif
4144
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03030000
4245
PyAPI_FUNC(PyObject *) PyImport_AddModuleObject(
4346
PyObject *name
4447
);
4548
#endif
49+
#ifndef Py_LIMITED_API
50+
PyAPI_FUNC(PyObject *) _PyImport_AddModuleObject(PyObject *, PyObject *);
51+
#endif
4652
PyAPI_FUNC(PyObject *) PyImport_AddModule(
4753
const char *name /* UTF-8 encoded string */
4854
);
@@ -92,14 +98,19 @@ PyAPI_FUNC(int) _PyImport_ReleaseLock(void);
9298
PyAPI_FUNC(void) _PyImport_ReInitLock(void);
9399

94100
PyAPI_FUNC(PyObject *) _PyImport_FindBuiltin(
95-
const char *name /* UTF-8 encoded string */
101+
const char *name, /* UTF-8 encoded string */
102+
PyObject *modules
96103
);
97104
PyAPI_FUNC(PyObject *) _PyImport_FindExtensionObject(PyObject *, PyObject *);
105+
PyAPI_FUNC(PyObject *) _PyImport_FindExtensionObjectEx(PyObject *, PyObject *,
106+
PyObject *);
98107
PyAPI_FUNC(int) _PyImport_FixupBuiltin(
99108
PyObject *mod,
100-
const char *name /* UTF-8 encoded string */
109+
const char *name, /* UTF-8 encoded string */
110+
PyObject *modules
101111
);
102-
PyAPI_FUNC(int) _PyImport_FixupExtensionObject(PyObject*, PyObject *, PyObject *);
112+
PyAPI_FUNC(int) _PyImport_FixupExtensionObject(PyObject*, PyObject *,
113+
PyObject *, PyObject *);
103114

104115
struct _inittab {
105116
const char *name; /* ASCII encoded string */

‎Include/modsupport.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ PyAPI_FUNC(int) PyModule_ExecDef(PyObject *module, PyModuleDef *def);
191191

192192
PyAPI_FUNC(PyObject *) PyModule_Create2(struct PyModuleDef*,
193193
int apiver);
194+
#ifndef Py_LIMITED_API
195+
PyAPI_FUNC(PyObject *) _PyModule_CreateInitialized(struct PyModuleDef*,
196+
int apiver);
197+
#endif
194198

195199
#ifdef Py_LIMITED_API
196200
#define PyModule_Create(module) \

‎Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Change direct usage of PyInterpreterState.modules to PyImport_GetModuleDict().
2+
Also introduce more uniformity in other code that deals with sys.modules.
3+
This helps reduce complications when working on sys.modules.

‎Objects/moduleobject.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,18 @@ _add_methods_to_object(PyObject *module, PyObject *name, PyMethodDef *functions)
161161

162162
PyObject *
163163
PyModule_Create2(struct PyModuleDef* module, int module_api_version)
164+
{
165+
if (!_PyImport_IsInitialized(PyThreadState_GET()->interp))
166+
Py_FatalError("Python import machinery not initialized");
167+
return _PyModule_CreateInitialized(module, module_api_version);
168+
}
169+
170+
PyObject *
171+
_PyModule_CreateInitialized(struct PyModuleDef* module, int module_api_version)
164172
{
165173
const char* name;
166174
PyModuleObject *m;
167-
PyInterpreterState *interp = PyThreadState_Get()->interp;
168-
if (interp->modules == NULL)
169-
Py_FatalError("Python import machinery not initialized");
175+
170176
if (!PyModuleDef_Init(module))
171177
return NULL;
172178
name = module->m_name;

‎Objects/typeobject.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3902,7 +3902,6 @@ import_copyreg(void)
39023902
{
39033903
PyObject *copyreg_str;
39043904
PyObject *copyreg_module;
3905-
PyInterpreterState *interp = PyThreadState_GET()->interp;
39063905
_Py_IDENTIFIER(copyreg);
39073906

39083907
copyreg_str = _PyUnicode_FromId(&PyId_copyreg);
@@ -3914,7 +3913,8 @@ import_copyreg(void)
39143913
by storing a reference to the cached module in a static variable, but
39153914
this broke when multiple embedded interpreters were in use (see issue
39163915
#17408 and #19088). */
3917-
copyreg_module = PyDict_GetItemWithError(interp->modules, copyreg_str);
3916+
PyObject *modules = PyImport_GetModuleDict();
3917+
copyreg_module = PyDict_GetItemWithError(modules, copyreg_str);
39183918
if (copyreg_module != NULL) {
39193919
Py_INCREF(copyreg_module);
39203920
return copyreg_module;

‎Python/bltinmodule.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2685,7 +2685,7 @@ _PyBuiltin_Init(void)
26852685
PyType_Ready(&PyZip_Type) < 0)
26862686
return NULL;
26872687

2688-
mod = PyModule_Create(&builtinsmodule);
2688+
mod = _PyModule_CreateInitialized(&builtinsmodule, PYTHON_API_VERSION);
26892689
if (mod == NULL)
26902690
return NULL;
26912691
dict = PyModule_GetDict(mod);

‎Python/import.c

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,17 @@ PyImport_GetModuleDict(void)
296296
return interp->modules;
297297
}
298298

299+
/* In some corner cases it is important to be sure that the import
300+
machinery has been initialized (or not cleaned up yet). For
301+
example, see issue #4236 and PyModule_Create2(). */
302+
303+
int
304+
_PyImport_IsInitialized(PyInterpreterState *interp)
305+
{
306+
if (interp->modules == NULL)
307+
return 0;
308+
return 1;
309+
}
299310

300311
/* List of names to clear in sys */
301312
static const char * const sys_deletes[] = {
@@ -323,7 +334,7 @@ PyImport_Cleanup(void)
323334
Py_ssize_t pos;
324335
PyObject *key, *value, *dict;
325336
PyInterpreterState *interp = PyThreadState_GET()->interp;
326-
PyObject *modules = interp->modules;
337+
PyObject *modules = PyImport_GetModuleDict();
327338
PyObject *weaklist = NULL;
328339
const char * const *p;
329340

@@ -511,9 +522,9 @@ PyImport_GetMagicTag(void)
511522

512523
int
513524
_PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
514-
PyObject *filename)
525+
PyObject *filename, PyObject *modules)
515526
{
516-
PyObject *modules, *dict, *key;
527+
PyObject *dict, *key;
517528
struct PyModuleDef *def;
518529
int res;
519530
if (extensions == NULL) {
@@ -530,7 +541,6 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
530541
PyErr_BadInternalCall();
531542
return -1;
532543
}
533-
modules = PyImport_GetModuleDict();
534544
if (PyDict_SetItem(modules, name, mod) < 0)
535545
return -1;
536546
if (_PyState_AddModule(mod, def) < 0) {
@@ -562,20 +572,28 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
562572
}
563573

564574
int
565-
_PyImport_FixupBuiltin(PyObject *mod, const char *name)
575+
_PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules)
566576
{
567577
int res;
568578
PyObject *nameobj;
569579
nameobj = PyUnicode_InternFromString(name);
570580
if (nameobj == NULL)
571581
return -1;
572-
res = _PyImport_FixupExtensionObject(mod, nameobj, nameobj);
582+
res = _PyImport_FixupExtensionObject(mod, nameobj, nameobj, modules);
573583
Py_DECREF(nameobj);
574584
return res;
575585
}
576586

577587
PyObject *
578588
_PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
589+
{
590+
PyObject *modules = PyImport_GetModuleDict();
591+
return _PyImport_FindExtensionObjectEx(name, filename, modules);
592+
}
593+
594+
PyObject *
595+
_PyImport_FindExtensionObjectEx(PyObject *name, PyObject *filename,
596+
PyObject *modules)
579597
{
580598
PyObject *mod, *mdict, *key;
581599
PyModuleDef* def;
@@ -592,7 +610,7 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
592610
/* Module does not support repeated initialization */
593611
if (def->m_base.m_copy == NULL)
594612
return NULL;
595-
mod = PyImport_AddModuleObject(name);
613+
mod = _PyImport_AddModuleObject(name, modules);
596614
if (mod == NULL)
597615
return NULL;
598616
mdict = PyModule_GetDict(mod);
@@ -607,14 +625,14 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
607625
mod = def->m_base.m_init();
608626
if (mod == NULL)
609627
return NULL;
610-
if (PyDict_SetItem(PyImport_GetModuleDict(), name, mod) == -1) {
628+
if (PyDict_SetItem(modules, name, mod) == -1) {
611629
Py_DECREF(mod);
612630
return NULL;
613631
}
614632
Py_DECREF(mod);
615633
}
616634
if (_PyState_AddModule(mod, def) < 0) {
617-
PyDict_DelItem(PyImport_GetModuleDict(), name);
635+
PyDict_DelItem(modules, name);
618636
Py_DECREF(mod);
619637
return NULL;
620638
}
@@ -626,13 +644,13 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
626644
}
627645

628646
PyObject *
629-
_PyImport_FindBuiltin(const char *name)
647+
_PyImport_FindBuiltin(const char *name, PyObject *modules)
630648
{
631649
PyObject *res, *nameobj;
632650
nameobj = PyUnicode_InternFromString(name);
633651
if (nameobj == NULL)
634652
return NULL;
635-
res = _PyImport_FindExtensionObject(nameobj, nameobj);
653+
res = _PyImport_FindExtensionObjectEx(nameobj, nameobj, modules);
636654
Py_DECREF(nameobj);
637655
return res;
638656
}
@@ -647,6 +665,12 @@ PyObject *
647665
PyImport_AddModuleObject(PyObject *name)
648666
{
649667
PyObject *modules = PyImport_GetModuleDict();
668+
return _PyImport_AddModuleObject(name, modules);
669+
}
670+
671+
PyObject *
672+
_PyImport_AddModuleObject(PyObject *name, PyObject *modules)
673+
{
650674
PyObject *m;
651675

652676
if ((m = PyDict_GetItemWithError(modules, name)) != NULL &&
@@ -1042,6 +1066,7 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
10421066
return NULL;
10431067
}
10441068

1069+
PyObject *modules = NULL;
10451070
for (p = PyImport_Inittab; p->name != NULL; p++) {
10461071
PyModuleDef *def;
10471072
if (_PyUnicode_EqualToASCIIString(name, p->name)) {
@@ -1067,7 +1092,11 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
10671092
return NULL;
10681093
}
10691094
def->m_base.m_init = p->initfunc;
1070-
if (_PyImport_FixupExtensionObject(mod, name, name) < 0) {
1095+
if (modules == NULL) {
1096+
modules = PyImport_GetModuleDict();
1097+
}
1098+
if (_PyImport_FixupExtensionObject(mod, name, name,
1099+
modules) < 0) {
10711100
Py_DECREF(name);
10721101
return NULL;
10731102
}
@@ -1511,7 +1540,8 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
15111540
Py_INCREF(abs_name);
15121541
}
15131542

1514-
mod = PyDict_GetItem(interp->modules, abs_name);
1543+
PyObject *modules = PyImport_GetModuleDict();
1544+
mod = PyDict_GetItem(modules, abs_name);
15151545
if (mod != NULL && mod != Py_None) {
15161546
_Py_IDENTIFIER(__spec__);
15171547
_Py_IDENTIFIER(_initializing);
@@ -1598,7 +1628,8 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
15981628
goto error;
15991629
}
16001630

1601-
final_mod = PyDict_GetItem(interp->modules, to_return);
1631+
PyObject *modules = PyImport_GetModuleDict();
1632+
final_mod = PyDict_GetItem(modules, to_return);
16021633
Py_DECREF(to_return);
16031634
if (final_mod == NULL) {
16041635
PyErr_Format(PyExc_KeyError,

‎Python/importdl.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
215215
else
216216
Py_INCREF(path);
217217

218-
if (_PyImport_FixupExtensionObject(m, name_unicode, path) < 0)
218+
PyObject *modules = PyImport_GetModuleDict();
219+
if (_PyImport_FixupExtensionObject(m, name_unicode, path, modules) < 0)
219220
goto error;
220221

221222
Py_DECREF(name_unicode);

‎Python/pylifecycle.c

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -675,9 +675,20 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
675675
if (!_PyFloat_Init())
676676
Py_FatalError("Py_InitializeCore: can't init float");
677677

678-
interp->modules = PyDict_New();
679-
if (interp->modules == NULL)
678+
PyObject *modules = PyDict_New();
679+
if (modules == NULL)
680680
Py_FatalError("Py_InitializeCore: can't make modules dictionary");
681+
interp->modules = modules;
682+
683+
sysmod = _PySys_BeginInit();
684+
if (sysmod == NULL)
685+
Py_FatalError("Py_InitializeCore: can't initialize sys");
686+
interp->sysdict = PyModule_GetDict(sysmod);
687+
if (interp->sysdict == NULL)
688+
Py_FatalError("Py_InitializeCore: can't initialize sys dict");
689+
Py_INCREF(interp->sysdict);
690+
PyDict_SetItemString(interp->sysdict, "modules", modules);
691+
_PyImport_FixupBuiltin(sysmod, "sys", modules);
681692

682693
/* Init Unicode implementation; relies on the codec registry */
683694
if (_PyUnicode_Init() < 0)
@@ -689,7 +700,7 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
689700
bimod = _PyBuiltin_Init();
690701
if (bimod == NULL)
691702
Py_FatalError("Py_InitializeCore: can't initialize builtins modules");
692-
_PyImport_FixupBuiltin(bimod, "builtins");
703+
_PyImport_FixupBuiltin(bimod, "builtins", modules);
693704
interp->builtins = PyModule_GetDict(bimod);
694705
if (interp->builtins == NULL)
695706
Py_FatalError("Py_InitializeCore: can't initialize builtins dict");
@@ -698,17 +709,6 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
698709
/* initialize builtin exceptions */
699710
_PyExc_Init(bimod);
700711

701-
sysmod = _PySys_BeginInit();
702-
if (sysmod == NULL)
703-
Py_FatalError("Py_InitializeCore: can't initialize sys");
704-
interp->sysdict = PyModule_GetDict(sysmod);
705-
if (interp->sysdict == NULL)
706-
Py_FatalError("Py_InitializeCore: can't initialize sys dict");
707-
Py_INCREF(interp->sysdict);
708-
_PyImport_FixupBuiltin(sysmod, "sys");
709-
PyDict_SetItemString(interp->sysdict, "modules",
710-
interp->modules);
711-
712712
/* Set up a preliminary stderr printer until we have enough
713713
infrastructure for the io module in place. */
714714
pstderr = PyFile_NewStdPrinter(fileno(stderr));
@@ -1211,9 +1211,23 @@ Py_NewInterpreter(void)
12111211

12121212
/* XXX The following is lax in error checking */
12131213

1214-
interp->modules = PyDict_New();
1214+
PyObject *modules = PyDict_New();
1215+
if (modules == NULL)
1216+
Py_FatalError("Py_NewInterpreter: can't make modules dictionary");
1217+
interp->modules = modules;
12151218

1216-
bimod = _PyImport_FindBuiltin("builtins");
1219+
sysmod = _PyImport_FindBuiltin("sys", modules);
1220+
if (sysmod != NULL) {
1221+
interp->sysdict = PyModule_GetDict(sysmod);
1222+
if (interp->sysdict == NULL)
1223+
goto handle_error;
1224+
Py_INCREF(interp->sysdict);
1225+
PyDict_SetItemString(interp->sysdict, "modules", modules);
1226+
PySys_SetPath(Py_GetPath());
1227+
_PySys_EndInit(interp->sysdict);
1228+
}
1229+
1230+
bimod = _PyImport_FindBuiltin("builtins", modules);
12171231
if (bimod != NULL) {
12181232
interp->builtins = PyModule_GetDict(bimod);
12191233
if (interp->builtins == NULL)
@@ -1224,18 +1238,9 @@ Py_NewInterpreter(void)
12241238
/* initialize builtin exceptions */
12251239
_PyExc_Init(bimod);
12261240

1227-
sysmod = _PyImport_FindBuiltin("sys");
12281241
if (bimod != NULL && sysmod != NULL) {
12291242
PyObject *pstderr;
12301243

1231-
interp->sysdict = PyModule_GetDict(sysmod);
1232-
if (interp->sysdict == NULL)
1233-
goto handle_error;
1234-
Py_INCREF(interp->sysdict);
1235-
_PySys_EndInit(interp->sysdict);
1236-
PySys_SetPath(Py_GetPath());
1237-
PyDict_SetItemString(interp->sysdict, "modules",
1238-
interp->modules);
12391244
/* Set up a preliminary stderr printer until we have enough
12401245
infrastructure for the io module in place. */
12411246
pstderr = PyFile_NewStdPrinter(fileno(stderr));
@@ -1911,9 +1916,8 @@ wait_for_thread_shutdown(void)
19111916
{
19121917
_Py_IDENTIFIER(_shutdown);
19131918
PyObject *result;
1914-
PyThreadState *tstate = PyThreadState_GET();
1915-
PyObject *threading = PyMapping_GetItemString(tstate->interp->modules,
1916-
"threading");
1919+
PyObject *modules = PyImport_GetModuleDict();
1920+
PyObject *threading = PyMapping_GetItemString(modules, "threading");
19171921
if (threading == NULL) {
19181922
/* threading not imported */
19191923
PyErr_Clear();

0 commit comments

Comments
 (0)