Add PyExc_ImportCycleError and raise it when a cycle is detected

Author: DinoV

Include/internal/pycore_import.h

@@ -57,7 +57,6 @@ _PyImport_LazyImportModuleLevelObject(PyThreadState *tstate, PyObject *name, PyO
 #define IMPORTS_INIT \
     { \
         DLOPENFLAGS_INIT \
-        .lazy_import_resolution_depth = 0, \
         .find_and_load = { \
             .header = 1, \
         }, \

Include/internal/pycore_interp_structs.h

@@ -316,8 +316,7 @@ struct _import_state {
     PyObject *lazy_import_func;
     int lazy_imports_mode;
     PyObject *lazy_imports_filter;
-    /* Counter to prevent recursive lazy import creation */
-    int lazy_import_resolution_depth;
+    PyObject *lazy_importing_modules;
     /* The global import lock. */
     _PyRecursiveMutex lock;
     /* diagnostic info in PyImport_ImportModuleLevelObject() */

Include/pyerrors.h

@@ -91,6 +91,9 @@ PyAPI_DATA(PyObject *) PyExc_EOFError;
 PyAPI_DATA(PyObject *) PyExc_FloatingPointError;
 PyAPI_DATA(PyObject *) PyExc_OSError;
 PyAPI_DATA(PyObject *) PyExc_ImportError;
+#if !defined(Py_LIMITED_API)
+PyAPI_DATA(PyObject *) PyExc_ImportCycleError;
+#endif
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000
 PyAPI_DATA(PyObject *) PyExc_ModuleNotFoundError;
 #endif

Lib/_compat_pickle.py

@@ -240,6 +240,7 @@
     REVERSE_NAME_MAPPING[('builtins', excname)] = ('exceptions', 'OSError')
 
 PYTHON3_IMPORTERROR_EXCEPTIONS = (
+    'ImportCycleError',
     'ModuleNotFoundError',
 )
 

Lib/test/exception_hierarchy.txt

@@ -14,6 +14,7 @@ BaseException
       ├── EOFError
       ├── ExceptionGroup [BaseExceptionGroup]
       ├── ImportError
+      │    └── ImportCycleError
       │    └── ModuleNotFoundError
       ├── LookupError
       │    ├── IndexError

Lib/test/test_import/data/lazy_imports/eager_import_func.py

@@ -0,0 +1,3 @@
+def f():
+    import test.test_import.data.lazy_imports.basic2 as basic2
+    return basic2

Objects/exceptions.c

@@ -1958,6 +1958,12 @@ static PyTypeObject _PyExc_ImportError = {
 };
 PyObject *PyExc_ImportError = (PyObject *)&_PyExc_ImportError;
 
+/*
+ *    ImportCycleError extends ImportError
+ */
+
+MiddlingExtendsException(PyExc_ImportError, ImportCycleError, ImportError,
+                         "Import produces a cycle.");
 /*
  *    ModuleNotFoundError extends ImportError
  */
@@ -4391,6 +4397,7 @@ static struct static_exception static_exceptions[] = {
     {&_PyExc_IncompleteInputError, "_IncompleteInputError"}, // base: SyntaxError(Exception)
     ITEM(IndexError),  // base: LookupError(Exception)
     ITEM(KeyError),  // base: LookupError(Exception)
+    ITEM(ImportCycleError), // base: ImportError(Exception)
     ITEM(ModuleNotFoundError), // base: ImportError(Exception)
     ITEM(NotImplementedError),  // base: RuntimeError(Exception)
     ITEM(PythonFinalizationError),  // base: RuntimeError(Exception)
@@ -4586,4 +4593,3 @@ _PyException_AddNote(PyObject *exc, PyObject *note)
     Py_XDECREF(r);
     return res;
 }
-

Objects/moduleobject.c

@@ -1046,6 +1046,13 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
         if (PyLazyImport_CheckExact(attr)) {
             PyObject *new_value = _PyImport_LoadLazyImportTstate(PyThreadState_GET(), attr);
             if (new_value == NULL) {
+                if (suppress && PyErr_ExceptionMatches(PyExc_ImportCycleError)) {
+                    // ImportCycleError is raised when a lazy object tries to import itself.
+                    // In this case, the error should not propagate to the caller and
+                    // instead treated as if the attribute doesn't exist.
+                    PyErr_Clear();
+                }
+
                 return NULL;
             } else if (PyDict_SetItem(m->md_dict, name, new_value) < 0) {
                 Py_DECREF(new_value);

Python/import.c

@@ -3710,6 +3710,33 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
 
     PyLazyImportObject *lz = (PyLazyImportObject *)lazy_import;
 
+    // Check if we are already importing this module, if so, then we want to return an error
+    // that indicates we've hit a cycle which will indicate the value isn't yet available.
+    PyInterpreterState *interp = tstate->interp;
+    PyObject *importing = interp->imports.lazy_importing_modules;
+    if (importing == NULL) {
+        importing = interp->imports.lazy_importing_modules = PySet_New(NULL);
+        if (importing == NULL) {
+            return NULL;
+        }
+    }
+
+    int is_loading = PySet_Contains(importing, lazy_import);
+    if (is_loading < 0 ) {
+        return NULL;
+    } else if (is_loading == 1) {
+        PyObject *name = _PyLazyImport_GetName(lazy_import);
+        PyObject *errmsg = PyUnicode_FromFormat("cannot import name %R "
+                                                "(most likely due to a circular import)",
+                                                name);
+        PyErr_SetImportErrorSubclass(PyExc_ImportCycleError, errmsg, lz->lz_from, NULL);
+        Py_XDECREF(errmsg);
+        Py_XDECREF(name);
+        return NULL;
+    } else if (PySet_Add(importing, lazy_import) < 0) {
+        goto error;
+    }
+
     Py_ssize_t dot = -1;
     int full = 0;
     if (lz->lz_attr != NULL) {
@@ -3738,10 +3765,6 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
 
     PyObject *globals = PyEval_GetGlobals();
 
-    // Increment counter to prevent recursive lazy import creation
-    PyInterpreterState *interp = tstate->interp;
-    interp->imports.lazy_import_resolution_depth++;
-
     if (full) {
         obj = _PyEval_ImportNameWithImport(tstate,
                                    lz->lz_import_func,
@@ -3753,7 +3776,6 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
     } else {
         PyObject *name = PyUnicode_Substring(lz->lz_from, 0, dot);
         if (name == NULL) {
-            interp->imports.lazy_import_resolution_depth--;
             goto error;
         }
         obj = _PyEval_ImportNameWithImport(tstate,
@@ -3766,9 +3788,6 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
         Py_DECREF(name);
     }
 
-    // Decrement counter
-    interp->imports.lazy_import_resolution_depth--;
-
     if (obj == NULL) {
         goto error;
     }
@@ -3860,6 +3879,11 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
     }
 
 ok:
+    if (PySet_Discard(importing, lazy_import) < 0) {
+        Py_DECREF(obj);
+        obj = NULL;
+    }
+
     Py_XDECREF(fromlist);
     return obj;
 }
@@ -3950,8 +3974,7 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
         goto error;
     }
 
-    // Only check __lazy_modules__ if we're not already resolving a lazy import
-    if (interp->imports.lazy_import_resolution_depth == 0 && globals != NULL &&
+    if (globals != NULL &&
         PyMapping_GetOptionalItem(globals, &_Py_ID(__lazy_modules__), &lazy_modules) < 0) {
         goto error;
     }
@@ -3974,7 +3997,7 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
         goto error;
     }
 
-    if (interp->imports.lazy_import_resolution_depth == 0 && lazy_modules != NULL) {
+    if (lazy_modules != NULL) {
         // Check and see if the module is opting in w/o syntax for backwards compatibility
         // with older Python versions.
         int contains = PySequence_Contains(lazy_modules, name);