[Python-checkins] bpo-28411: Isolate PyInterpreterState.modules (#3575)

Eric Snow webhook-mailer at python.org
Thu Sep 14 14:18:15 EDT 2017

• Previous message (by thread): [Python-checkins] [3.6] bpo-31457: Allow for nested LoggerAdapter objects (GH-3551) (#3576)
• Next message (by thread): [Python-checkins] bpo-28556: typing.get_type_hints: better globalns for classes and modules (#3582)
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

https://github.com/python/cpython/commit/d393c1b227f22fb9af66040b2b367c99a4d1fa9a
commit: d393c1b227f22fb9af66040b2b367c99a4d1fa9a
branch: master
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017年09月14日T12:18:12-06:00
summary:
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.
files:
A Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst
M Include/import.h
M Include/modsupport.h
M Objects/moduleobject.c
M Objects/typeobject.c
M Python/bltinmodule.c
M Python/import.c
M Python/importdl.c
M Python/pylifecycle.c
M Python/sysmodule.c
diff --git a/Include/import.h b/Include/import.h
index 7e83985b3a4..95c52b0bfba 100644
--- a/Include/import.h
+++ b/Include/import.h
@@ -38,11 +38,17 @@ PyAPI_FUNC(PyObject *) PyImport_ExecCodeModuleObject(
 );
 #endif
 PyAPI_FUNC(PyObject *) PyImport_GetModuleDict(void);
+#ifndef Py_LIMITED_API
+PyAPI_FUNC(int) _PyImport_IsInitialized(PyInterpreterState *);
+#endif
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03030000
 PyAPI_FUNC(PyObject *) PyImport_AddModuleObject(
 PyObject *name
 );
 #endif
+#ifndef Py_LIMITED_API
+PyAPI_FUNC(PyObject *) _PyImport_AddModuleObject(PyObject *, PyObject *);
+#endif
 PyAPI_FUNC(PyObject *) PyImport_AddModule(
 const char *name /* UTF-8 encoded string */
 );
@@ -92,14 +98,19 @@ PyAPI_FUNC(int) _PyImport_ReleaseLock(void);
 PyAPI_FUNC(void) _PyImport_ReInitLock(void);
 
 PyAPI_FUNC(PyObject *) _PyImport_FindBuiltin(
- const char *name /* UTF-8 encoded string */
+ const char *name, /* UTF-8 encoded string */
+ PyObject *modules
 );
 PyAPI_FUNC(PyObject *) _PyImport_FindExtensionObject(PyObject *, PyObject *);
+PyAPI_FUNC(PyObject *) _PyImport_FindExtensionObjectEx(PyObject *, PyObject *,
+ PyObject *);
 PyAPI_FUNC(int) _PyImport_FixupBuiltin(
 PyObject *mod,
- const char *name /* UTF-8 encoded string */
+ const char *name, /* UTF-8 encoded string */
+ PyObject *modules
 );
-PyAPI_FUNC(int) _PyImport_FixupExtensionObject(PyObject*, PyObject *, PyObject *);
+PyAPI_FUNC(int) _PyImport_FixupExtensionObject(PyObject*, PyObject *,
+ PyObject *, PyObject *);
 
 struct _inittab {
 const char *name; /* ASCII encoded string */
diff --git a/Include/modsupport.h b/Include/modsupport.h
index 8c7cf39d9a3..73d86a94b95 100644
--- a/Include/modsupport.h
+++ b/Include/modsupport.h
@@ -191,6 +191,10 @@ PyAPI_FUNC(int) PyModule_ExecDef(PyObject *module, PyModuleDef *def);
 
 PyAPI_FUNC(PyObject *) PyModule_Create2(struct PyModuleDef*,
 int apiver);
+#ifndef Py_LIMITED_API
+PyAPI_FUNC(PyObject *) _PyModule_CreateInitialized(struct PyModuleDef*,
+ int apiver);
+#endif
 
 #ifdef Py_LIMITED_API
 #define PyModule_Create(module) \
diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst
new file mode 100644
index 00000000000..47f9fa684e5
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst	
@@ -0,0 +1,3 @@
+Change direct usage of PyInterpreterState.modules to PyImport_GetModuleDict().
+Also introduce more uniformity in other code that deals with sys.modules.
+This helps reduce complications when working on sys.modules.
diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index 5fab06d31df..2be49fbda38 100644
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -162,11 +162,17 @@ _add_methods_to_object(PyObject *module, PyObject *name, PyMethodDef *functions)
 PyObject *
 PyModule_Create2(struct PyModuleDef* module, int module_api_version)
 {
+ if (!_PyImport_IsInitialized(PyThreadState_GET()->interp))
+ Py_FatalError("Python import machinery not initialized");
+ return _PyModule_CreateInitialized(module, module_api_version);
+}
+
+PyObject *
+_PyModule_CreateInitialized(struct PyModuleDef* module, int module_api_version)
+{
 const char* name;
 PyModuleObject *m;
- PyInterpreterState *interp = PyThreadState_Get()->interp;
- if (interp->modules == NULL)
- Py_FatalError("Python import machinery not initialized");
+
 if (!PyModuleDef_Init(module))
 return NULL;
 name = module->m_name;
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index dc4d2edc5cf..9ebbb21ef8d 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3902,7 +3902,6 @@ import_copyreg(void)
 {
 PyObject *copyreg_str;
 PyObject *copyreg_module;
- PyInterpreterState *interp = PyThreadState_GET()->interp;
 _Py_IDENTIFIER(copyreg);
 
 copyreg_str = _PyUnicode_FromId(&PyId_copyreg);
@@ -3914,7 +3913,8 @@ import_copyreg(void)
 by storing a reference to the cached module in a static variable, but
 this broke when multiple embedded interpreters were in use (see issue
 #17408 and #19088). */
- copyreg_module = PyDict_GetItemWithError(interp->modules, copyreg_str);
+ PyObject *modules = PyImport_GetModuleDict();
+ copyreg_module = PyDict_GetItemWithError(modules, copyreg_str);
 if (copyreg_module != NULL) {
 Py_INCREF(copyreg_module);
 return copyreg_module;
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 5e1f1d3854f..c363cfe8cea 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -2685,7 +2685,7 @@ _PyBuiltin_Init(void)
 PyType_Ready(&PyZip_Type) < 0)
 return NULL;
 
- mod = PyModule_Create(&builtinsmodule);
+ mod = _PyModule_CreateInitialized(&builtinsmodule, PYTHON_API_VERSION);
 if (mod == NULL)
 return NULL;
 dict = PyModule_GetDict(mod);
diff --git a/Python/import.c b/Python/import.c
index 2aacf659351..7aa7a1bdf79 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -296,6 +296,17 @@ PyImport_GetModuleDict(void)
 return interp->modules;
 }
 
+/* In some corner cases it is important to be sure that the import
+ machinery has been initialized (or not cleaned up yet). For
+ example, see issue #4236 and PyModule_Create2(). */
+
+int
+_PyImport_IsInitialized(PyInterpreterState *interp)
+{
+ if (interp->modules == NULL)
+ return 0;
+ return 1;
+}
 
 /* List of names to clear in sys */
 static const char * const sys_deletes[] = {
@@ -323,7 +334,7 @@ PyImport_Cleanup(void)
 Py_ssize_t pos;
 PyObject *key, *value, *dict;
 PyInterpreterState *interp = PyThreadState_GET()->interp;
- PyObject *modules = interp->modules;
+ PyObject *modules = PyImport_GetModuleDict();
 PyObject *weaklist = NULL;
 const char * const *p;
 
@@ -511,9 +522,9 @@ PyImport_GetMagicTag(void)
 
 int
 _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
- PyObject *filename)
+ PyObject *filename, PyObject *modules)
 {
- PyObject *modules, *dict, *key;
+ PyObject *dict, *key;
 struct PyModuleDef *def;
 int res;
 if (extensions == NULL) {
@@ -530,7 +541,6 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
 PyErr_BadInternalCall();
 return -1;
 }
- modules = PyImport_GetModuleDict();
 if (PyDict_SetItem(modules, name, mod) < 0)
 return -1;
 if (_PyState_AddModule(mod, def) < 0) {
@@ -562,14 +572,14 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,
 }
 
 int
-_PyImport_FixupBuiltin(PyObject *mod, const char *name)
+_PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules)
 {
 int res;
 PyObject *nameobj;
 nameobj = PyUnicode_InternFromString(name);
 if (nameobj == NULL)
 return -1;
- res = _PyImport_FixupExtensionObject(mod, nameobj, nameobj);
+ res = _PyImport_FixupExtensionObject(mod, nameobj, nameobj, modules);
 Py_DECREF(nameobj);
 return res;
 }
@@ -577,6 +587,14 @@ _PyImport_FixupBuiltin(PyObject *mod, const char *name)
 PyObject *
 _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
 {
+ PyObject *modules = PyImport_GetModuleDict();
+ return _PyImport_FindExtensionObjectEx(name, filename, modules);
+}
+
+PyObject *
+_PyImport_FindExtensionObjectEx(PyObject *name, PyObject *filename,
+ PyObject *modules)
+{
 PyObject *mod, *mdict, *key;
 PyModuleDef* def;
 if (extensions == NULL)
@@ -592,7 +610,7 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
 /* Module does not support repeated initialization */
 if (def->m_base.m_copy == NULL)
 return NULL;
- mod = PyImport_AddModuleObject(name);
+ mod = _PyImport_AddModuleObject(name, modules);
 if (mod == NULL)
 return NULL;
 mdict = PyModule_GetDict(mod);
@@ -607,14 +625,14 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
 mod = def->m_base.m_init();
 if (mod == NULL)
 return NULL;
- if (PyDict_SetItem(PyImport_GetModuleDict(), name, mod) == -1) {
+ if (PyDict_SetItem(modules, name, mod) == -1) {
 Py_DECREF(mod);
 return NULL;
 }
 Py_DECREF(mod);
 }
 if (_PyState_AddModule(mod, def) < 0) {
- PyDict_DelItem(PyImport_GetModuleDict(), name);
+ PyDict_DelItem(modules, name);
 Py_DECREF(mod);
 return NULL;
 }
@@ -626,13 +644,13 @@ _PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
 }
 
 PyObject *
-_PyImport_FindBuiltin(const char *name)
+_PyImport_FindBuiltin(const char *name, PyObject *modules)
 {
 PyObject *res, *nameobj;
 nameobj = PyUnicode_InternFromString(name);
 if (nameobj == NULL)
 return NULL;
- res = _PyImport_FindExtensionObject(nameobj, nameobj);
+ res = _PyImport_FindExtensionObjectEx(nameobj, nameobj, modules);
 Py_DECREF(nameobj);
 return res;
 }
@@ -647,6 +665,12 @@ PyObject *
 PyImport_AddModuleObject(PyObject *name)
 {
 PyObject *modules = PyImport_GetModuleDict();
+ return _PyImport_AddModuleObject(name, modules);
+}
+
+PyObject *
+_PyImport_AddModuleObject(PyObject *name, PyObject *modules)
+{
 PyObject *m;
 
 if ((m = PyDict_GetItemWithError(modules, name)) != NULL &&
@@ -1042,6 +1066,7 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
 return NULL;
 }
 
+ PyObject *modules = NULL;
 for (p = PyImport_Inittab; p->name != NULL; p++) {
 PyModuleDef *def;
 if (_PyUnicode_EqualToASCIIString(name, p->name)) {
@@ -1067,7 +1092,11 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
 return NULL;
 }
 def->m_base.m_init = p->initfunc;
- if (_PyImport_FixupExtensionObject(mod, name, name) < 0) {
+ if (modules == NULL) {
+ modules = PyImport_GetModuleDict();
+ }
+ if (_PyImport_FixupExtensionObject(mod, name, name,
+ modules) < 0) {
 Py_DECREF(name);
 return NULL;
 }
@@ -1511,7 +1540,8 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
 Py_INCREF(abs_name);
 }
 
- mod = PyDict_GetItem(interp->modules, abs_name);
+ PyObject *modules = PyImport_GetModuleDict();
+ mod = PyDict_GetItem(modules, abs_name);
 if (mod != NULL && mod != Py_None) {
 _Py_IDENTIFIER(__spec__);
 _Py_IDENTIFIER(_initializing);
@@ -1598,7 +1628,8 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
 goto error;
 }
 
- final_mod = PyDict_GetItem(interp->modules, to_return);
+ PyObject *modules = PyImport_GetModuleDict();
+ final_mod = PyDict_GetItem(modules, to_return);
 Py_DECREF(to_return);
 if (final_mod == NULL) {
 PyErr_Format(PyExc_KeyError,
diff --git a/Python/importdl.c b/Python/importdl.c
index d8656b94333..32fb7e1be21 100644
--- a/Python/importdl.c
+++ b/Python/importdl.c
@@ -215,7 +215,8 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
 else
 Py_INCREF(path);
 
- if (_PyImport_FixupExtensionObject(m, name_unicode, path) < 0)
+ PyObject *modules = PyImport_GetModuleDict();
+ if (_PyImport_FixupExtensionObject(m, name_unicode, path, modules) < 0)
 goto error;
 
 Py_DECREF(name_unicode);
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 2aac901ad13..5c8cf5b9bd5 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -675,9 +675,20 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
 if (!_PyFloat_Init())
 Py_FatalError("Py_InitializeCore: can't init float");
 
- interp->modules = PyDict_New();
- if (interp->modules == NULL)
+ PyObject *modules = PyDict_New();
+ if (modules == NULL)
 Py_FatalError("Py_InitializeCore: can't make modules dictionary");
+ interp->modules = modules;
+
+ sysmod = _PySys_BeginInit();
+ if (sysmod == NULL)
+ Py_FatalError("Py_InitializeCore: can't initialize sys");
+ interp->sysdict = PyModule_GetDict(sysmod);
+ if (interp->sysdict == NULL)
+ Py_FatalError("Py_InitializeCore: can't initialize sys dict");
+ Py_INCREF(interp->sysdict);
+ PyDict_SetItemString(interp->sysdict, "modules", modules);
+ _PyImport_FixupBuiltin(sysmod, "sys", modules);
 
 /* Init Unicode implementation; relies on the codec registry */
 if (_PyUnicode_Init() < 0)
@@ -689,7 +700,7 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
 bimod = _PyBuiltin_Init();
 if (bimod == NULL)
 Py_FatalError("Py_InitializeCore: can't initialize builtins modules");
- _PyImport_FixupBuiltin(bimod, "builtins");
+ _PyImport_FixupBuiltin(bimod, "builtins", modules);
 interp->builtins = PyModule_GetDict(bimod);
 if (interp->builtins == NULL)
 Py_FatalError("Py_InitializeCore: can't initialize builtins dict");
@@ -698,17 +709,6 @@ void _Py_InitializeCore(const _PyCoreConfig *config)
 /* initialize builtin exceptions */
 _PyExc_Init(bimod);
 
- sysmod = _PySys_BeginInit();
- if (sysmod == NULL)
- Py_FatalError("Py_InitializeCore: can't initialize sys");
- interp->sysdict = PyModule_GetDict(sysmod);
- if (interp->sysdict == NULL)
- Py_FatalError("Py_InitializeCore: can't initialize sys dict");
- Py_INCREF(interp->sysdict);
- _PyImport_FixupBuiltin(sysmod, "sys");
- PyDict_SetItemString(interp->sysdict, "modules",
- interp->modules);
-
 /* Set up a preliminary stderr printer until we have enough
 infrastructure for the io module in place. */
 pstderr = PyFile_NewStdPrinter(fileno(stderr));
@@ -1211,9 +1211,23 @@ Py_NewInterpreter(void)
 
 /* XXX The following is lax in error checking */
 
- interp->modules = PyDict_New();
+ PyObject *modules = PyDict_New();
+ if (modules == NULL)
+ Py_FatalError("Py_NewInterpreter: can't make modules dictionary");
+ interp->modules = modules;
 
- bimod = _PyImport_FindBuiltin("builtins");
+ sysmod = _PyImport_FindBuiltin("sys", modules);
+ if (sysmod != NULL) {
+ interp->sysdict = PyModule_GetDict(sysmod);
+ if (interp->sysdict == NULL)
+ goto handle_error;
+ Py_INCREF(interp->sysdict);
+ PyDict_SetItemString(interp->sysdict, "modules", modules);
+ PySys_SetPath(Py_GetPath());
+ _PySys_EndInit(interp->sysdict);
+ }
+
+ bimod = _PyImport_FindBuiltin("builtins", modules);
 if (bimod != NULL) {
 interp->builtins = PyModule_GetDict(bimod);
 if (interp->builtins == NULL)
@@ -1224,18 +1238,9 @@ Py_NewInterpreter(void)
 /* initialize builtin exceptions */
 _PyExc_Init(bimod);
 
- sysmod = _PyImport_FindBuiltin("sys");
 if (bimod != NULL && sysmod != NULL) {
 PyObject *pstderr;
 
- interp->sysdict = PyModule_GetDict(sysmod);
- if (interp->sysdict == NULL)
- goto handle_error;
- Py_INCREF(interp->sysdict);
- _PySys_EndInit(interp->sysdict);
- PySys_SetPath(Py_GetPath());
- PyDict_SetItemString(interp->sysdict, "modules",
- interp->modules);
 /* Set up a preliminary stderr printer until we have enough
 infrastructure for the io module in place. */
 pstderr = PyFile_NewStdPrinter(fileno(stderr));
@@ -1911,9 +1916,8 @@ wait_for_thread_shutdown(void)
 {
 _Py_IDENTIFIER(_shutdown);
 PyObject *result;
- PyThreadState *tstate = PyThreadState_GET();
- PyObject *threading = PyMapping_GetItemString(tstate->interp->modules,
- "threading");
+ PyObject *modules = PyImport_GetModuleDict();
+ PyObject *threading = PyMapping_GetItemString(modules, "threading");
 if (threading == NULL) {
 /* threading not imported */
 PyErr_Clear();
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index 9e13d49417d..d463683df1d 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -162,8 +162,9 @@ static PyObject *
 sys_displayhook(PyObject *self, PyObject *o)
 {
 PyObject *outf;
- PyInterpreterState *interp = PyThreadState_GET()->interp;
- PyObject *modules = interp->modules;
+ PyObject *modules = PyImport_GetModuleDict();
+ if (modules == NULL)
+ return NULL;
 PyObject *builtins;
 static PyObject *newline = NULL;
 int err;
@@ -1949,7 +1950,7 @@ _PySys_BeginInit(void)
 PyObject *m, *sysdict, *version_info;
 int res;
 
- m = PyModule_Create(&sysmodule);
+ m = _PyModule_CreateInitialized(&sysmodule, PYTHON_API_VERSION);
 if (m == NULL)
 return NULL;
 sysdict = PyModule_GetDict(m);

---

• Previous message (by thread): [Python-checkins] [3.6] bpo-31457: Allow for nested LoggerAdapter objects (GH-3551) (#3576)
• Next message (by thread): [Python-checkins] bpo-28556: typing.get_type_hints: better globalns for classes and modules (#3582)
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

More information about the Python-checkins mailing list