Skip to content

Commit aab59a0

Browse files
brittanyreypablogsal
authored andcommitted
Remove resolved names from sys.lazy_modules consistently
Names only left sys.lazy_modules through _imp._set_lazy_attributes(), which the import machinery calls from _find_and_load_unlocked(). Two cases never reached it, so their names were recorded and then kept forever: - A lazy import of a module already in sys.modules. _find_and_load() returns early, so nothing ever discards the name. Do not record it in the first place. - The "pkg.attr" entry for `lazy from pkg import attr`. The import machinery only discards module names, and attr is often not a module. Discard it when the lazy object is reified, where the name is already known and has been resolved either way. Submodules that are not yet loaded are still tracked: loading a package does not load its submodules, so those imports can still fire. Names whose reification failed also stay tracked, since the import can still happen.
1 parent 1e8ff18 commit aab59a0

3 files changed

Lines changed: 143 additions & 7 deletions

File tree

‎Lib/test/test_lazy_import/__init__.py‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ def test_sys_lazy_modules(self):
5353
self.fail('lazy import failed')
5454

5555
self.assertFalse("test.test_lazy_import.data.basic2" in sys.modules)
56-
self.assertIn("test.test_lazy_import.data", sys.lazy_modules)
56+
# The package is already loaded, so it is not a pending import.
57+
self.assertNotIn("test.test_lazy_import.data", sys.lazy_modules)
5758
self.assertIn("test.test_lazy_import.data.basic2", sys.lazy_modules)
5859
test.test_lazy_import.data.basic_from_unused.basic2
5960
self.assertNotIn("test.test_import.data", sys.lazy_modules)
@@ -1275,6 +1276,103 @@ def test_lazy_module_without_children_is_tracked(self):
12751276
""")
12761277
assert_python_ok("-c", code)
12771278

1279+
def test_already_loaded_module_is_not_tracked(self):
1280+
"""A lazy import of an already loaded module should not be tracked."""
1281+
code = textwrap.dedent("""
1282+
import sys
1283+
1284+
# Loaded by a regular import.
1285+
import json
1286+
lazy import json as lazy_json
1287+
assert "json" not in sys.lazy_modules, (
1288+
f"expected 'json' not in sys.lazy_modules, got {sys.lazy_modules}"
1289+
)
1290+
1291+
# Loaded by reifying an earlier lazy import.
1292+
lazy import base64
1293+
_ = base64.b64encode
1294+
lazy import base64 as lazy_base64
1295+
assert "base64" not in sys.lazy_modules, (
1296+
f"expected 'base64' not in sys.lazy_modules, got {sys.lazy_modules}"
1297+
)
1298+
""")
1299+
assert_python_ok("-c", code)
1300+
1301+
def test_already_loaded_submodule_is_not_tracked(self):
1302+
"""`lazy from` a loaded submodule should not be tracked either."""
1303+
code = textwrap.dedent("""
1304+
import sys
1305+
import test.test_lazy_import.data.pkg.b
1306+
lazy from test.test_lazy_import.data.pkg import b
1307+
assert "test.test_lazy_import.data.pkg.b" not in sys.lazy_modules, (
1308+
f"expected 'pkg.b' untracked, got {sys.lazy_modules}"
1309+
)
1310+
""")
1311+
assert_python_ok("-c", code)
1312+
1313+
def test_attribute_entry_removed_on_reification(self):
1314+
"""`lazy from x import attr` should untrack "x.attr" once resolved."""
1315+
code = textwrap.dedent("""
1316+
import sys
1317+
lazy from test.test_lazy_import.data.basic2 import x
1318+
assert "test.test_lazy_import.data.basic2.x" in sys.lazy_modules, (
1319+
f"expected 'basic2.x' tracked, got {sys.lazy_modules}"
1320+
)
1321+
_ = x
1322+
assert "test.test_lazy_import.data.basic2.x" not in sys.lazy_modules, (
1323+
f"expected 'basic2.x' untracked, got {sys.lazy_modules}"
1324+
)
1325+
""")
1326+
assert_python_ok("-c", code)
1327+
1328+
def test_failed_reification_stays_tracked(self):
1329+
"""A lazy import that fails to resolve must stay tracked."""
1330+
code = textwrap.dedent("""
1331+
import sys
1332+
lazy import test.test_lazy_import.data.broken_module
1333+
try:
1334+
_ = test.test_lazy_import.data.broken_module
1335+
except ValueError:
1336+
pass
1337+
else:
1338+
raise AssertionError("ValueError was not raised")
1339+
assert "test.test_lazy_import.data.broken_module" in sys.lazy_modules, (
1340+
f"failed reification must stay tracked, got {sys.lazy_modules}"
1341+
)
1342+
""")
1343+
assert_python_ok("-c", code)
1344+
1345+
def test_blocked_module_is_still_tracked(self):
1346+
"""A ``None`` entry in sys.modules must not count as loaded."""
1347+
code = textwrap.dedent("""
1348+
import sys
1349+
sys.modules['test.test_lazy_import.data.basic2'] = None
1350+
lazy import test.test_lazy_import.data.basic2
1351+
assert "test.test_lazy_import.data.basic2" in sys.lazy_modules, (
1352+
f"blocked module must stay tracked, got {sys.lazy_modules}"
1353+
)
1354+
""")
1355+
assert_python_ok("-c", code)
1356+
1357+
def test_pending_submodule_is_still_tracked(self):
1358+
"""`lazy from` a submodule that is not loaded must stay tracked."""
1359+
code = textwrap.dedent("""
1360+
import sys
1361+
lazy from test.test_lazy_import.data.pkg import b
1362+
assert "test.test_lazy_import.data.pkg.b" in sys.lazy_modules, (
1363+
f"expected 'pkg.b' tracked, got {sys.lazy_modules}"
1364+
)
1365+
import test.test_lazy_import.data.pkg
1366+
assert "test.test_lazy_import.data.pkg.b" not in sys.modules, (
1367+
"loading the package must not load the submodule"
1368+
)
1369+
assert "test.test_lazy_import.data.pkg.b" in sys.lazy_modules, (
1370+
f"loading the package must not untrack the submodule, "
1371+
f"got {sys.lazy_modules}"
1372+
)
1373+
""")
1374+
assert_python_ok("-c", code)
1375+
12781376

12791377
@support.requires_subprocess()
12801378
class CommandLineAndEnvVarTests(unittest.TestCase):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Resolved names are now removed from :data:`sys.lazy_modules` more
2+
consistently: a lazy import of an already loaded module is no longer recorded,
3+
and reifying ``lazy from pkg import attr`` now discards the ``pkg.attr`` entry.

‎Python/import.c‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3917,6 +3917,20 @@ lazy_import_replay_from(PyThreadState *tstate, PyObject *mod,
39173917
return obj;
39183918
}
39193919

3920+
// The import machinery only discards module names, so this is what removes
3921+
// the "pkg.attr" entry left by `lazy from pkg import attr`, submodule or not.
3922+
static int
3923+
discard_reified_lazy_import(PyInterpreterState *interp, PyObject *lazy_import)
3924+
{
3925+
PyObject *name = _PyLazyImport_GetName(lazy_import);
3926+
if (name == NULL) {
3927+
return -1;
3928+
}
3929+
int res = PySet_Discard(LAZY_MODULES(interp), name);
3930+
Py_DECREF(name);
3931+
return res < 0 ? -1 : 0;
3932+
}
3933+
39203934
PyObject *
39213935
_PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
39223936
{
@@ -4100,6 +4114,10 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
41004114
}
41014115

41024116
ok:
4117+
if (obj != NULL && discard_reified_lazy_import(interp, lazy_import) < 0) {
4118+
Py_CLEAR(obj);
4119+
}
4120+
41034121
if (PySet_Discard(importing, lazy_import) < 0) {
41044122
Py_CLEAR(obj);
41054123
}
@@ -4358,6 +4376,27 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
43584376
return final_mod;
43594377
}
43604378

4379+
// Check if a module is already loaded before adding it to sys.lazy_modules
4380+
static int
4381+
lazy_modules_add(PyThreadState *tstate, PyObject *name)
4382+
{
4383+
PyObject *modules = get_modules_dict(tstate, false);
4384+
if (modules == NULL) {
4385+
return -1;
4386+
}
4387+
PyObject *existing;
4388+
if (PyDict_GetItemRef(modules, name, &existing) < 0) {
4389+
return -1;
4390+
}
4391+
// A None entry blocks the import rather than satisfying it.
4392+
int loaded = (existing != NULL && existing != Py_None);
4393+
Py_XDECREF(existing);
4394+
if (loaded) {
4395+
return 0;
4396+
}
4397+
return PySet_Add(LAZY_MODULES(tstate->interp), name);
4398+
}
4399+
43614400
// ensure we have the set for the parent module name in sys.lazy_modules.
43624401
// Returns a new reference.
43634402
static PyObject *
@@ -4451,9 +4490,7 @@ register_from_lazy_on_parent(PyThreadState *tstate, PyObject *abs_name,
44514490
return -1;
44524491
}
44534492

4454-
// Add the module name to sys.lazy_modules set (PEP 810).
4455-
PyObject *lazy_modules = LAZY_MODULES(tstate->interp);
4456-
if (PySet_Add(lazy_modules, fromname) < 0) {
4493+
if (lazy_modules_add(tstate, fromname) < 0) {
44574494
Py_DECREF(fromname);
44584495
return -1;
44594496
}
@@ -4627,9 +4664,7 @@ _PyImport_LazyImportModuleLevelObject(PyThreadState *tstate,
46274664
return NULL;
46284665
}
46294666

4630-
// Add the module name to sys.lazy_modules set (PEP 810).
4631-
PyObject *lazy_modules = LAZY_MODULES(tstate->interp);
4632-
if (PySet_Add(lazy_modules, abs_name) < 0) {
4667+
if (lazy_modules_add(tstate, abs_name) < 0) {
46334668
goto error;
46344669
}
46354670

0 commit comments

Comments
 (0)