diff --git a/Lib/test/test_lazy_import/__init__.py b/Lib/test/test_lazy_import/__init__.py index 5026c9670d81d2..f9ef8ce2cfef8c 100644 --- a/Lib/test/test_lazy_import/__init__.py +++ b/Lib/test/test_lazy_import/__init__.py @@ -13,7 +13,7 @@ import contextlib from test import support -from test.support.script_helper import assert_python_ok +from test.support.script_helper import assert_python_ok, assert_python_failure try: import _testcapi @@ -617,6 +617,14 @@ def test_dunder_lazy_import_invalid_arguments(self): with self.assertRaises(TypeError): __lazy_import__("sys", globals=1) + code = textwrap.dedent(""" + __lazy_import__("sys", fromlist=(1, 2, 3)) + """) + result = assert_python_failure("-c", code, NO_COLOR='y') + self.assertIn( + b"TypeError: Item in ``from list'' must be str, not int", + result.err) + def test_dunder_lazy_import_builtins(self): """__lazy_import__ should use module's __builtins__ for __import__.""" from test.test_lazy_import.data import dunder_lazy_import_builtins @@ -766,6 +774,28 @@ def test_missing_lazy_from_import_shows_chained_traceback(self): """) assert_python_ok("-c", code) + @support.subTests('name', ( + 'test.test_lazy_import.data.broken_module_chained_cause', + 'test.test_lazy_import.data.broken_module_chained_context', + 'test.test_lazy_import.data.broken_module_chained_suppressed', + )) + def test_chained_exception_import_shows_notes(self, name): + """Accessing missing attribute from lazy from-import should chain errors.""" + code = textwrap.dedent(f""" + lazy import {name} + + try: + _ = test + except ValueError as e: + assert any( + note.startswith("lazy import of '{name}' declared in ") + for note in e.__notes__ + ), e.__notes__ + else: + raise AssertionError("ImportError was not raised") + """) + assert_python_ok("-c", code) + def test_reification_retries_on_failure(self): """Failed reification should allow retry on subsequent access. diff --git a/Lib/test/test_lazy_import/data/broken_module_chained_cause.py b/Lib/test/test_lazy_import/data/broken_module_chained_cause.py new file mode 100644 index 00000000000000..786607b7bb557e --- /dev/null +++ b/Lib/test/test_lazy_import/data/broken_module_chained_cause.py @@ -0,0 +1,3 @@ +# Module that raises an exception with explicit cause during import +cause = ValueError("Cause of failure") +raise ValueError("This module always fails to import") from cause diff --git a/Lib/test/test_lazy_import/data/broken_module_chained_context.py b/Lib/test/test_lazy_import/data/broken_module_chained_context.py new file mode 100644 index 00000000000000..82640856e3e2d8 --- /dev/null +++ b/Lib/test/test_lazy_import/data/broken_module_chained_context.py @@ -0,0 +1,5 @@ +# Module that raises an exception with context during import +try: + raise ValueError("Cause of failure") +except: + raise ValueError("This module always fails to import") diff --git a/Lib/test/test_lazy_import/data/broken_module_chained_suppressed.py b/Lib/test/test_lazy_import/data/broken_module_chained_suppressed.py new file mode 100644 index 00000000000000..5c34654afc13bf --- /dev/null +++ b/Lib/test/test_lazy_import/data/broken_module_chained_suppressed.py @@ -0,0 +1,2 @@ +# Module that raises an exception with suppressed context during import +raise ValueError("This module always fails to import") from None diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 57874de6ee7497..c38d867875bdaa 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2014,9 +2014,11 @@ static void replace_value(PyDictObject *mp, PyObject *key, Py_ssize_t ix, PyObject *old_value, PyObject *value) { + assert(can_modify_dict(mp)); + assert(old_value != NULL); + if (old_value != value) { _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, mp, key, value); - assert(old_value != NULL); if (DK_IS_UNICODE(mp->ma_keys)) { if (_PyDict_HasSplitTable(mp)) { STORE_SPLIT_VALUE(mp, ix, value); @@ -2031,7 +2033,9 @@ replace_value(PyDictObject *mp, PyObject *key, Py_ssize_t ix, STORE_VALUE(ep, value); } } - Py_DECREF(old_value); /* which **CAN** re-enter (see issue #22653) */ + Py_DECREF(old_value); /* which **CAN** re-enter (see gh-66843) */ + + ASSERT_CONSISTENT(mp); } /* @@ -2082,7 +2086,6 @@ insertdict(PyDictObject *mp, } replace_value(mp, key, ix, old_value, value); - ASSERT_CONSISTENT(mp); Py_DECREF(key); return 0; @@ -3082,7 +3085,9 @@ _PyDict_ReplaceItemIf(PyObject *op, PyObject *key, PyObject *expected, PyObject *replacement) { assert(PyDict_Check(op)); - assert(expected != NULL && replacement != NULL); + assert(expected != NULL); + assert(replacement != NULL); + Py_hash_t hash = PyObject_Hash(key); if (hash == -1) { return -1; @@ -3098,7 +3103,6 @@ _PyDict_ReplaceItemIf(PyObject *op, PyObject *key, else if (current == expected) { // Do not look up the key again: equality can execute Python code. replace_value(mp, key, ix, current, Py_NewRef(replacement)); - ASSERT_CONSISTENT(mp); result = 1; } Py_END_CRITICAL_SECTION(); diff --git a/Objects/lazyimportobject.c b/Objects/lazyimportobject.c index 72624b746fe64c..e4a949a5456cfd 100644 --- a/Objects/lazyimportobject.c +++ b/Objects/lazyimportobject.c @@ -16,8 +16,10 @@ typedef struct { PyObject_HEAD PyObject *lz_builtins; // Roots own the mapping; projections retain the root. - // A root stores its absolute name and original fromlist. A projection - // stores its source placeholder and the attribute to import from it. + // A root stores its absolute name (PyUnicode) in lz_from, and original + // fromlist in lz_attr. + // A projection stores its source placeholder (a PyLazyImportObject) + // in lz_from, and the attribute to import from (PyUnicode) it in lz_attr. PyObject *lz_from; PyObject *lz_attr; // Declaration location. @@ -45,9 +47,18 @@ _PyLazyImport_New(_PyInterpreterFrame *frame, PyObject *builtins, "lazy_import: fromlist must be None, a string, or a tuple"); return NULL; } - assert(PyLazyImport_CheckExact(name) ? builtins == NULL : builtins != NULL); - assert(!PyLazyImport_CheckExact(name) || - (fromlist != NULL && PyUnicode_Check(fromlist))); +#ifndef NDEBUG + if (PyLazyImport_CheckExact(name)) { + // projection + assert(builtins == NULL); + assert(fromlist != NULL); + assert(PyUnicode_Check(fromlist)); + } + else { + // root + assert(builtins != NULL); + } +#endif PyLazyImportObject *m = PyObject_GC_New( PyLazyImportObject, &PyLazyImport_Type); if (m == NULL) { @@ -71,6 +82,7 @@ _PyLazyImport_New(_PyInterpreterFrame *frame, PyObject *builtins, // Reuse concrete attributes of initialized modules without waiting for imports // or resolving lazy attributes. Failed cache lookups are retried at resolution. +// May return NULL with or without an exception set. static PyObject * lazy_import_get_loaded_attr(PyThreadState *tstate, PyObject *name, PyObject *attr_name) @@ -466,6 +478,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import) // Loading pkg.child can replace a placeholder in pkg.child with the module // before a from-import retrieves the value that belongs in that binding. +// This is an optimization that can be safely skipped. static int lazy_import_replace_child(PyThreadState *tstate, PyObject *placeholder, PyObject *name, PyObject *namespace, @@ -486,9 +499,9 @@ lazy_import_replace_child(PyThreadState *tstate, PyObject *placeholder, if (end - dot - 1 != PyUnicode_GET_LENGTH(name)) { return 0; } - int matches = PyUnicode_Tailmatch(root->lz_from, name, dot + 1, end, 1); + Py_ssize_t matches = PyUnicode_Tailmatch(root->lz_from, name, dot + 1, end, 1); if (matches <= 0) { - return matches; + return matches ? -1 : 0; } PyObject *parent_name = PyUnicode_Substring(root->lz_from, 0, dot); if (parent_name == NULL) { @@ -563,7 +576,7 @@ lazy_import_resolve(PyObject *self, PyObject *args) static PyMethodDef lazy_import_methods[] = { { "resolve", lazy_import_resolve, METH_NOARGS, - PyDoc_STR("resolves the lazy import and returns the actual object") + PyDoc_STR("Resolve the lazy import and return the imported object.") }, {NULL, NULL} };