From d393c1b227f22fb9af66040b2b367c99a4d1fa9a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 14 Sep 2017 12:18:12 -0600 Subject: 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. --- Include/import.h | 17 ++++-- Include/modsupport.h | 4 ++ .../2017-09-11-09-24-21.bpo-28411.12SpAm.rst | 3 ++ Objects/moduleobject.c | 12 +++-- Objects/typeobject.c | 4 +- Python/bltinmodule.c | 2 +- Python/import.c | 59 ++++++++++++++++----- Python/importdl.c | 3 +- Python/pylifecycle.c | 60 ++++++++++++---------- Python/sysmodule.c | 7 +-- 10 files changed, 116 insertions(+), 55 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-11-09-24-21.bpo-28411.12SpAm.rst diff --git a/Include/import.h b/Include/import.h index 7e83985..95c52b0 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 8c7cf39..73d86a9 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 0000000..47f9fa6 --- /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 5fab06d..2be49fb 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 dc4d2ed..9ebbb21 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 5e1f1d3..c363cfe 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 2aacf65..7aa7a1b 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 d8656b9..32fb7e1 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 2aac901..5c8cf5b 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 9e13d49..d463683 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); -- cgit v0.12