From 8684e59c5cccd5aac7b1f582048a78f37b9b8a82 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 6 Apr 2018 18:35:59 +0300 Subject: [PATCH 1/8] bpo-33237: Improve AttributeError message for partially initialized module. --- Objects/moduleobject.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 5fad4474be0f7c..4d4f4de7aee107 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -718,8 +718,36 @@ module_getattro(PyModuleObject *m, PyObject *name) _Py_IDENTIFIER(__name__); mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__); if (mod_name && PyUnicode_Check(mod_name)) { - PyErr_Format(PyExc_AttributeError, - "module '%U' has no attribute '%U'", mod_name, name); + _Py_IDENTIFIER(__spec__); + _Py_IDENTIFIER(_initializing); + PyObject *value = NULL; + PyObject *spec; + int initializing = 0; + Py_INCREF(mod_name); + spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__); + if (spec != NULL && spec != Py_None) { + value = _PyObject_GetAttrId(spec, &PyId__initializing); + } + if (value == NULL) + PyErr_Clear(); + else { + initializing = PyObject_IsTrue(value); + Py_DECREF(value); + if (initializing < 0) + PyErr_Clear(); + } + if (initializing > 0) { + PyErr_Format(PyExc_AttributeError, + "partially initialized " + "module '%U' has no attribute '%U'", + mod_name, name); + } + else { + PyErr_Format(PyExc_AttributeError, + "module '%U' has no attribute '%U'", + mod_name, name); + } + Py_DECREF(mod_name); return NULL; } } From b072b9badd0be2fbc8a4e3c27f520be4cec0f55c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 6 Apr 2018 19:04:30 +0300 Subject: [PATCH 2/8] Update error message. --- Objects/moduleobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 4d4f4de7aee107..8e38db6ff76d3b 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -739,7 +739,8 @@ module_getattro(PyModuleObject *m, PyObject *name) if (initializing > 0) { PyErr_Format(PyExc_AttributeError, "partially initialized " - "module '%U' has no attribute '%U'", + "module '%U' has no attribute '%U' " + "(most likely due to a circular import)", mod_name, name); } else { From 770f2f5e62b8f4bece4dcf9e69c22d6fe0efa2eb Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 24 Jul 2018 12:01:09 +0300 Subject: [PATCH 3/8] Add _PyModule_IsInitializing(). --- Include/moduleobject.h | 1 + Objects/moduleobject.c | 46 +++++++++++++++++++++++++----------------- Python/import.c | 30 ++++++--------------------- 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 1d8fe46dea03a1..bd210ebfca8eff 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -30,6 +30,7 @@ PyAPI_FUNC(PyObject *) PyModule_GetFilenameObject(PyObject *); #ifndef Py_LIMITED_API PyAPI_FUNC(void) _PyModule_Clear(PyObject *); PyAPI_FUNC(void) _PyModule_ClearDict(PyObject *); +PyAPI_FUNC(int) _PyModule_IsInitializing(PyObject *); #endif PyAPI_FUNC(struct PyModuleDef*) PyModule_GetDef(PyObject*); PyAPI_FUNC(void*) PyModule_GetState(PyObject*); diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 8e38db6ff76d3b..2ffde3a70baec5 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -699,6 +699,33 @@ module_repr(PyModuleObject *m) return PyObject_CallMethod(interp->importlib, "_module_repr", "O", m); } +int +_PyModule_IsInitializing(PyObject *m) +{ + if (PyModule_Check(m) && ((PyModuleObject *)m)->md_dict) { + _Py_IDENTIFIER(__spec__); + _Py_IDENTIFIER(_initializing); + PyObject *value = NULL; + PyObject *spec; + int initializing = 0; + spec = _PyDict_GetItemId(((PyModuleObject *)m)->md_dict, &PyId___spec__); + if (spec != NULL && spec != Py_None) { + value = _PyObject_GetAttrId(spec, &PyId__initializing); + } + if (value == NULL) { + PyErr_Clear(); + } + else { + initializing = PyObject_IsTrue(value); + Py_DECREF(value); + if (initializing < 0) + PyErr_Clear(); + return initializing > 0; + } + } + return 0; +} + static PyObject* module_getattro(PyModuleObject *m, PyObject *name) { @@ -718,25 +745,8 @@ module_getattro(PyModuleObject *m, PyObject *name) _Py_IDENTIFIER(__name__); mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__); if (mod_name && PyUnicode_Check(mod_name)) { - _Py_IDENTIFIER(__spec__); - _Py_IDENTIFIER(_initializing); - PyObject *value = NULL; - PyObject *spec; - int initializing = 0; Py_INCREF(mod_name); - spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__); - if (spec != NULL && spec != Py_None) { - value = _PyObject_GetAttrId(spec, &PyId__initializing); - } - if (value == NULL) - PyErr_Clear(); - else { - initializing = PyObject_IsTrue(value); - Py_DECREF(value); - if (initializing < 0) - PyErr_Clear(); - } - if (initializing > 0) { + if (_PyModule_IsInitializing((PyObject *)m)) { PyErr_Format(PyExc_AttributeError, "partially initialized " "module '%U' has no attribute '%U' " diff --git a/Python/import.c b/Python/import.c index 3a591836654b31..b564128bc27e1e 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1720,38 +1720,20 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, mod = PyImport_GetModule(abs_name); if (mod != NULL && mod != Py_None) { - _Py_IDENTIFIER(__spec__); - _Py_IDENTIFIER(_initializing); _Py_IDENTIFIER(_lock_unlock_module); - PyObject *value = NULL; - PyObject *spec; - int initializing = 0; /* Optimization: only call _bootstrap._lock_unlock_module() if __spec__._initializing is true. NOTE: because of this, initializing must be set *before* stuffing the new module in sys.modules. */ - spec = _PyObject_GetAttrId(mod, &PyId___spec__); - if (spec != NULL) { - value = _PyObject_GetAttrId(spec, &PyId__initializing); - Py_DECREF(spec); - } - if (value == NULL) - PyErr_Clear(); - else { - initializing = PyObject_IsTrue(value); + if (_PyModule_IsInitializing(mod)) { + PyObject *value = _PyObject_CallMethodIdObjArgs(interp->importlib, + &PyId__lock_unlock_module, abs_name, + NULL); + if (value == NULL) + goto error; Py_DECREF(value); - if (initializing == -1) - PyErr_Clear(); - if (initializing > 0) { - value = _PyObject_CallMethodIdObjArgs(interp->importlib, - &PyId__lock_unlock_module, abs_name, - NULL); - if (value == NULL) - goto error; - Py_DECREF(value); - } } } else { From 8df640697bb59f511ebb13823bb2f20b272ecaa8 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 24 Jul 2018 12:55:09 +0300 Subject: [PATCH 4/8] Add a news entry. --- .../Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst b/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst new file mode 100644 index 00000000000000..04bd86c3dd6a39 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst @@ -0,0 +1 @@ +Improved :exc:`AttributeError` message for partially initialized module. From 4c2dea3b64ed592c9c5c068bb611ac244c94ae71 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 28 Jul 2018 09:35:59 +0300 Subject: [PATCH 5/8] Refactoring. --- Objects/moduleobject.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 2ffde3a70baec5..b8a49cd035159c 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -702,12 +702,11 @@ module_repr(PyModuleObject *m) int _PyModule_IsInitializing(PyObject *m) { + _Py_IDENTIFIER(__spec__); + _Py_IDENTIFIER(_initializing); + PyObject *spec; + PyObject *value = NULL; if (PyModule_Check(m) && ((PyModuleObject *)m)->md_dict) { - _Py_IDENTIFIER(__spec__); - _Py_IDENTIFIER(_initializing); - PyObject *value = NULL; - PyObject *spec; - int initializing = 0; spec = _PyDict_GetItemId(((PyModuleObject *)m)->md_dict, &PyId___spec__); if (spec != NULL && spec != Py_None) { value = _PyObject_GetAttrId(spec, &PyId__initializing); @@ -716,10 +715,11 @@ _PyModule_IsInitializing(PyObject *m) PyErr_Clear(); } else { - initializing = PyObject_IsTrue(value); + int initializing = PyObject_IsTrue(value); Py_DECREF(value); - if (initializing < 0) + if (initializing < 0) { PyErr_Clear(); + } return initializing > 0; } } From 9f37fba2ce60611983d4cfabab48fde8abcfbc29 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 21 Oct 2018 00:05:41 +0300 Subject: [PATCH 6/8] Restore support of non-module subclasses in PyImport_ImportModuleLevelObject(). --- Include/moduleobject.h | 2 +- Objects/moduleobject.c | 31 +++++++++++++------------------ Python/import.c | 10 ++++++++-- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index bd210ebfca8eff..4d173808738cc9 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -30,7 +30,7 @@ PyAPI_FUNC(PyObject *) PyModule_GetFilenameObject(PyObject *); #ifndef Py_LIMITED_API PyAPI_FUNC(void) _PyModule_Clear(PyObject *); PyAPI_FUNC(void) _PyModule_ClearDict(PyObject *); -PyAPI_FUNC(int) _PyModule_IsInitializing(PyObject *); +PyAPI_FUNC(int) _PyModuleSpec_IsInitializing(PyObject *); #endif PyAPI_FUNC(struct PyModuleDef*) PyModule_GetDef(PyObject*); PyAPI_FUNC(void*) PyModule_GetState(PyObject*); diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 21995f6ed4cb44..8fdc9865633d04 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -699,29 +699,20 @@ module_repr(PyModuleObject *m) } int -_PyModule_IsInitializing(PyObject *m) +_PyModuleSpec_IsInitializing(PyObject *spec) { - _Py_IDENTIFIER(__spec__); - _Py_IDENTIFIER(_initializing); - PyObject *spec; - PyObject *value = NULL; - if (PyModule_Check(m) && ((PyModuleObject *)m)->md_dict) { - spec = _PyDict_GetItemId(((PyModuleObject *)m)->md_dict, &PyId___spec__); - if (spec != NULL && spec != Py_None) { - value = _PyObject_GetAttrId(spec, &PyId__initializing); - } - if (value == NULL) { - PyErr_Clear(); - } - else { + if (spec != NULL && spec != Py_None) { + _Py_IDENTIFIER(_initializing); + PyObject *value = _PyObject_GetAttrId(spec, &PyId__initializing); + if (value != NULL) { int initializing = PyObject_IsTrue(value); Py_DECREF(value); - if (initializing < 0) { - PyErr_Clear(); + if (initializing >= 0) { + return initializing; } - return initializing > 0; } } + PyErr_Clear(); return 0; } @@ -744,8 +735,11 @@ module_getattro(PyModuleObject *m, PyObject *name) _Py_IDENTIFIER(__name__); mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__); if (mod_name && PyUnicode_Check(mod_name)) { + _Py_IDENTIFIER(__spec__); Py_INCREF(mod_name); - if (_PyModule_IsInitializing((PyObject *)m)) { + PyObject *spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__); + Py_XINCREF(spec); + if (_PyModuleSpec_IsInitializing(spec)) { PyErr_Format(PyExc_AttributeError, "partially initialized " "module '%U' has no attribute '%U' " @@ -757,6 +751,7 @@ module_getattro(PyModuleObject *m, PyObject *name) "module '%U' has no attribute '%U'", mod_name, name); } + Py_XDECREF(spec); Py_DECREF(mod_name); return NULL; } diff --git a/Python/import.c b/Python/import.c index d92b1c60d9b8b1..c912814e2a9f05 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1719,21 +1719,27 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, mod = PyImport_GetModule(abs_name); if (mod != NULL && mod != Py_None) { + _Py_IDENTIFIER(__spec__); _Py_IDENTIFIER(_lock_unlock_module); + PyObject *spec; /* Optimization: only call _bootstrap._lock_unlock_module() if __spec__._initializing is true. NOTE: because of this, initializing must be set *before* stuffing the new module in sys.modules. */ - if (_PyModule_IsInitializing(mod)) { + spec = _PyObject_GetAttrId(mod, &PyId___spec__); + if (_PyModuleSpec_IsInitializing(spec)) { PyObject *value = _PyObject_CallMethodIdObjArgs(interp->importlib, &PyId__lock_unlock_module, abs_name, NULL); - if (value == NULL) + if (value == NULL) { + Py_DECREF(spec); goto error; + } Py_DECREF(value); } + Py_XDECREF(spec); } else { Py_XDECREF(mod); From 1bd96f9292bf1745596b0aaead00b7ad712f4425 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 21 Oct 2018 09:08:28 +0300 Subject: [PATCH 7/8] Add tests. --- Lib/test/test_import/__init__.py | 13 +++++++++++++ .../test_import/data/circular_imports/source.py | 2 ++ Lib/test/test_import/data/circular_imports/use.py | 2 ++ 3 files changed, 17 insertions(+) create mode 100644 Lib/test/test_import/data/circular_imports/source.py create mode 100644 Lib/test/test_import/data/circular_imports/use.py diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index fb9453ad0b3920..7306e0f7f722dd 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1271,6 +1271,19 @@ def test_binding(self): except ImportError: self.fail('circular import with binding a submodule to a name failed') + def test_crossreference1(self): + import test.test_import.data.circular_imports.use + import test.test_import.data.circular_imports.source + + def test_crossreference2(self): + with self.assertRaises(AttributeError) as cm: + import test.test_import.data.circular_imports.source + errmsg = str(cm.exception) + self.assertIn('test.test_import.data.circular_imports.source', errmsg) + self.assertIn('spam', errmsg) + self.assertIn('partially initialized module', errmsg) + self.assertIn('circular import', errmsg) + if __name__ == '__main__': # Test needs to be a package, so we can do relative imports. diff --git a/Lib/test/test_import/data/circular_imports/source.py b/Lib/test/test_import/data/circular_imports/source.py new file mode 100644 index 00000000000000..f104099048c547 --- /dev/null +++ b/Lib/test/test_import/data/circular_imports/source.py @@ -0,0 +1,2 @@ +from . import use +spam = 1 diff --git a/Lib/test/test_import/data/circular_imports/use.py b/Lib/test/test_import/data/circular_imports/use.py new file mode 100644 index 00000000000000..418f9e2688e9f6 --- /dev/null +++ b/Lib/test/test_import/data/circular_imports/use.py @@ -0,0 +1,2 @@ +from . import source +source.spam From 9a78720187bd8a1e6311cb5167d478fc0ea87a8f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 29 Oct 2018 20:37:54 +0200 Subject: [PATCH 8/8] Remove redundant condition and add a comment. --- Objects/moduleobject.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 8fdc9865633d04..5181941ee7bcfa 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -698,10 +698,13 @@ module_repr(PyModuleObject *m) return PyObject_CallMethod(interp->importlib, "_module_repr", "O", m); } +/* Check if the "_initializing" attribute of the module spec is set to true. + Clear the exception and return 0 if spec is NULL. + */ int _PyModuleSpec_IsInitializing(PyObject *spec) { - if (spec != NULL && spec != Py_None) { + if (spec != NULL) { _Py_IDENTIFIER(_initializing); PyObject *value = _PyObject_GetAttrId(spec, &PyId__initializing); if (value != NULL) {