Skip to content

Commit 141835b

Browse files
brittanyreymiss-islington
authored andcommitted
gh-158820: Add null checks to avoid segfaults with lazy imports at shutdown (GH-158821)
Destructors can run after finalization has cleared the interpreter's lazy_modules set and lazy_pending_submodules dict. Declaring or resolving a lazy import from one of them passed NULL to PySet_Add(), PySet_Discard() or the pending-submodules lookup. Treat the bookkeeping as a no-op once the state is gone, as the other readers of this state already do. (cherry picked from commit a6e6361) Co-authored-by: Brittany Reynoso <breynoso@meta.com>
1 parent cc3c98b commit 141835b

3 files changed

Lines changed: 69 additions & 3 deletions

File tree

‎Lib/test/test_lazy_import/__init__.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2822,5 +2822,60 @@ def test_fails_without_lazy(self):
28222822
self.assertIn("ImportError", result.stderr)
28232823

28242824

2825+
@support.requires_subprocess()
2826+
class LazyImportFinalizationTests(unittest.TestCase):
2827+
"""Destructors that use lazy imports after finalization freed their state."""
2828+
2829+
def test_declare(self):
2830+
code = textwrap.dedent("""
2831+
import builtins, os
2832+
2833+
class Canary:
2834+
def __del__(self, write=os.write, exec=exec,
2835+
builtins={"__lazy_import__": builtins.__lazy_import__}):
2836+
exec("lazy import a.b", {"__builtins__": builtins})
2837+
write(1, b"ok")
2838+
2839+
# Only this placeholder keeps the canary alive.
2840+
exec("lazy import pkg.mod", {"__builtins__": {
2841+
"__lazy_import__": builtins.__lazy_import__, "c": Canary()}})
2842+
""")
2843+
self.assertEqual(assert_python_ok("-c", code).out, b"ok")
2844+
2845+
def test_resolve(self):
2846+
code = textwrap.dedent("""
2847+
import contextvars, json, os, types
2848+
2849+
def safe_import(name, *args, allowed={"json": json}):
2850+
return allowed[name]
2851+
2852+
# Not in sys.modules, with builtins that still work at shutdown.
2853+
ns = types.ModuleType("ns")
2854+
ns.json = __lazy_import__("json", {"__builtins__": {"__import__": safe_import}})
2855+
2856+
class Canary:
2857+
def __del__(self, write=os.write, type=type, ns=ns):
2858+
write(1, type(ns.json).__name__.encode())
2859+
2860+
contextvars.ContextVar("v").set(Canary())
2861+
""")
2862+
self.assertEqual(assert_python_ok("-c", code).out, b"module")
2863+
2864+
def test_set_lazy_attributes(self):
2865+
code = textwrap.dedent("""
2866+
import _imp, os, sys
2867+
2868+
class Canary:
2869+
def __del__(self, write=os.write,
2870+
set_lazy=_imp._set_lazy_attributes):
2871+
set_lazy(None, "mod")
2872+
write(1, b"ok")
2873+
2874+
# Freed while sys.lazy_modules is being cleared.
2875+
sys.lazy_modules.add(Canary())
2876+
""")
2877+
self.assertEqual(assert_python_ok("-c", code).out, b"ok")
2878+
2879+
28252880
if __name__ == '__main__':
28262881
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a crash when a :ref:`lazy import <lazy-imports>` is declared or resolved
2+
by a finalizer running during interpreter shutdown.

‎Python/import.c‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,9 @@ _PyImport_ClearLazyModules(PyInterpreterState *interp)
289289
int
290290
_PyImport_DiscardLazyModule(PyInterpreterState *interp, PyObject *name)
291291
{
292+
if (LAZY_MODULES(interp) == NULL) {
293+
return 0;
294+
}
292295
return PySet_Discard(LAZY_MODULES(interp), name);
293296
}
294297

@@ -4176,7 +4179,10 @@ lazy_modules_add(PyThreadState *tstate, PyObject *name,
41764179
else {
41774180
Py_XDECREF(mod);
41784181
}
4179-
return loaded ? 0 : PySet_Add(LAZY_MODULES(tstate->interp), name);
4182+
if (loaded || LAZY_MODULES(tstate->interp) == NULL) {
4183+
return 0;
4184+
}
4185+
return PySet_Add(LAZY_MODULES(tstate->interp), name);
41804186
}
41814187

41824188
// Ensure a dict of pending submodule names exists for the parent.
@@ -4210,7 +4216,10 @@ register_lazy_on_parent(PyThreadState *tstate, PyObject *name, PyObject *source)
42104216
{
42114217
PyDictObject *pending =
42124218
(PyDictObject *)LAZY_PENDING_SUBMODULES(tstate->interp);
4213-
assert(pending != NULL);
4219+
// Finalizers can still run after finalize_modules() cleared the dict.
4220+
if (pending == NULL) {
4221+
return 0;
4222+
}
42144223
Py_ssize_t end = PyUnicode_GET_LENGTH(name);
42154224
while (true) {
42164225
Py_ssize_t dot = PyUnicode_FindChar(name, '.', 0, end, -1);
@@ -5572,7 +5581,7 @@ _imp__set_lazy_attributes_impl(PyObject *module, PyObject *modobj,
55725581
/*[clinic end generated code: output=3369bb3242b1f043 input=900339e013ab2b82]*/
55735582
{
55745583
PyInterpreterState *interp = _PyInterpreterState_GET();
5575-
if (PySet_Discard(LAZY_MODULES(interp), name) < 0) {
5584+
if (_PyImport_DiscardLazyModule(interp, name) < 0) {
55765585
return NULL;
55775586
}
55785587
if (_PyImport_ClearLazySubmodule(_PyThreadState_GET(), name, 0) < 0) {

0 commit comments

Comments
 (0)