Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Include/internal/pycore_import.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ extern PyObject * _PyImport_GetAbsName(
// Symbol is exported for the JIT on Windows builds.
PyAPI_FUNC(PyObject *) _PyImport_LoadLazyImportTstate(
PyThreadState *tstate, PyObject *lazy_import);
extern PyObject * _PyImport_ResolveLazyImportFromAttr(
PyThreadState *tstate, PyObject *package_name,
PyObject *name, PyObject *attr);
extern PyObject * _PyImport_TryLoadLazySubmodule(
PyObject *mod_name, PyObject *attr_name);
extern PyObject * _PyImport_LazyImportModuleLevelObject(
Expand Down
30 changes: 30 additions & 0 deletions Lib/test/test_lazy_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,36 @@ def test_package_from_import_with_module_getattr(self):
""")
assert_python_ok("-c", code)

@support.requires_subprocess()
def test_lazy_import_reimports_submodule_evicted_from_sys_modules(self):
"""A submodule evicted from sys.modules but still cached on its parent
must be re-imported, like eager 'import a.b.c as t' is."""
code = textwrap.dedent("""
import sys
modname = "test.test_lazy_import.data.pkg.bar"
import test.test_lazy_import.data.pkg.bar
del sys.modules[modname]
lazy import test.test_lazy_import.data.pkg.bar as t
t.f()
assert modname in sys.modules, modname
""")
assert_python_ok("-c", code)

@support.requires_subprocess()
def test_lazy_from_import_reimports_submodule_evicted_from_sys_modules(self):
"""Same as above for 'from a.b import c': the stale parent attribute
must not shadow a re-import after sys.modules eviction."""
code = textwrap.dedent("""
import sys
modname = "test.test_lazy_import.data.pkg.bar"
import test.test_lazy_import.data.pkg.bar
del sys.modules[modname]
lazy from test.test_lazy_import.data.pkg import bar
bar.f()
assert modname in sys.modules, modname
""")
assert_python_ok("-c", code)


class DunderLazyImportTests(LazyImportTestCase):
"""Tests for __lazy_import__ builtin function."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix lazy submodule imports so package submodules removed from
:data:`sys.modules` are properly re-imported.
9 changes: 6 additions & 3 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -3301,8 +3301,8 @@ _PyEval_LazyImportFrom(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObje
PyLazyImportObject *d = (PyLazyImportObject *)v;
PyObject *mod = PyImport_GetModule(d->lz_from);
if (mod != NULL) {
// Check if the module already has the attribute, if so, resolve it
// eagerly.
/* If the parent is already imported, resolve any module-valued
attribute through the same stale-submodule check. */
if (PyModule_Check(mod)) {
PyObject *mod_dict = PyModule_GetDict(mod);
if (mod_dict != NULL) {
Expand All @@ -3311,8 +3311,11 @@ _PyEval_LazyImportFrom(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObje
return NULL;
}
if (ret != NULL) {
PyObject *resolved = _PyImport_ResolveLazyImportFromAttr(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-import runs while executing the lazy from statement, before bar is accessed. We should keep a lazy proxy here and repair it during reification.

tstate, d->lz_from, name, ret);
Py_DECREF(ret);
Py_DECREF(mod);
return ret;
return resolved;
}
}
}
Expand Down
85 changes: 84 additions & 1 deletion Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -3885,6 +3885,76 @@ _PyImport_ResolveName(PyThreadState *tstate, PyObject *name,
return resolve_name(tstate, name, globals, level);
}

PyObject *
_PyImport_ResolveLazyImportFromAttr(PyThreadState *tstate,
PyObject *package_name,
PyObject *name, PyObject *attr)
{
/* Lazy from-imports repair evicted package submodules without
changing eager from-import behavior. */
if (!PyModule_Check(attr) || package_name == NULL
|| !PyUnicode_Check(package_name)) {
return Py_NewRef(attr);
}

PyObject *package = PyImport_GetModule(package_name);
if (package == NULL) {
if (_PyErr_Occurred(tstate)) {
return NULL;
}
return Py_NewRef(attr);
}

int is_package = PyObject_HasAttrWithError(package, &_Py_ID(__path__));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: do we have a test for the non-package branch? We dropped test_from_import_module_attr_from_non_package in the last commit, so the is_package <= 0 and matches <= 0 paths here look uncovered now. Maybe add a small lazy from test where the parent is a plain module exposing a module-valued attribute.

Py_DECREF(package);
if (is_package <= 0) {
if (is_package < 0) {
return NULL;
}
return Py_NewRef(attr);
}

PyObject *fullmodname = PyUnicode_FromFormat("%U.%U", package_name, name);
if (fullmodname == NULL) {
return NULL;
}

PyObject *attr_name;
if (PyObject_GetOptionalAttr(attr, &_Py_ID(__name__), &attr_name) < 0) {
Py_DECREF(fullmodname);
return NULL;
}

int matches = (attr_name != NULL && PyUnicode_Check(attr_name))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__name__ == fullmodname does not prove this is an importable submodule. Packages can export synthetic modules, so this turns a valid from-import into ModuleNotFoundError.

? PyObject_RichCompareBool(attr_name, fullmodname, Py_EQ)
: 0;
Py_XDECREF(attr_name);
if (matches <= 0) {
Py_DECREF(fullmodname);
return matches < 0 ? NULL : Py_NewRef(attr);
}

PyObject *submod = import_get_module(tstate, fullmodname);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can return a module that is still initializing in another thread. We need the initialization wait and recheck from PyImport_GetModule().

if (submod == NULL && !_PyErr_Occurred(tstate)) {
PyObject *imported = PyImport_ImportModuleLevelObject(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bypasses the __import__ captured by the lazy object, so custom import hooks are ignored. We need to pass the captured hook through this path.

fullmodname, NULL, NULL, NULL, 0);
if (imported == NULL) {
Py_DECREF(fullmodname);
return NULL;
}
Py_DECREF(imported);
submod = import_get_module(tstate, fullmodname);
}
Py_DECREF(fullmodname);
if (submod != NULL) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys.modules[fullmodname] can be None. This returns it as the binding instead of raising ModuleNotFoundError; we need to handle the sentinel explicitly.

return submod;
}
if (_PyErr_Occurred(tstate)) {
return NULL;
}
return Py_NewRef(attr);
}

PyObject *
_PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
{
Expand Down Expand Up @@ -4004,7 +4074,20 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)

if (lz->lz_attr != NULL && PyUnicode_Check(lz->lz_attr)) {
PyObject *from = obj;
obj = _PyEval_ImportFrom(tstate, from, lz->lz_attr);
obj = NULL;
PyObject *attr;
if (PyObject_GetOptionalAttr(from, lz->lz_attr, &attr) < 0) {
Py_DECREF(from);
goto error;
}
if (attr != NULL) {
obj = _PyImport_ResolveLazyImportFromAttr(
tstate, lz->lz_from, lz->lz_attr, attr);
Py_DECREF(attr);
}
else {
obj = _PyEval_ImportFrom(tstate, from, lz->lz_attr);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit: on the not-found path we look up lz->lz_attr here and then _PyEval_ImportFrom looks it up again, so a package __getattr__ runs twice, no? Not a blocker, but worth a comment if we keep it.

}
Py_DECREF(from);
if (obj == NULL) {
goto error;
Expand Down
Loading