gh-152298: Clarify lazy resolve owner cleanup

1sgtpepper · 1sgtpepper · commit 3ce4bd116fe1 · 2026-06-27T09:55:33.000+08:00
diff --git a/Doc/library/types.rst b/Doc/library/types.rst
@@ -350,6 +350,16 @@ Standard names are defined for the following types:
    actually accessed. This type can be used to detect lazy imports
    programmatically.
 
+   .. method:: resolve()
+
+      Resolve the lazy import and return the imported object. Successful
+      resolutions are cached.
+
+      For module-level lazy imports, ``resolve()`` replaces the original global
+      only while it still refers to this proxy. If that binding was deleted or
+      rebound, it is left unchanged and later ``resolve()`` calls on the same
+      proxy do not update it.
+
    .. versionadded:: 3.15
 
    .. seealso:: :pep:`810`
diff --git a/Include/internal/pycore_lazyimportobject.h b/Include/internal/pycore_lazyimportobject.h
@@ -19,6 +19,8 @@ typedef struct {
     PyObject *lz_builtins;
     PyObject *lz_from;
     PyObject *lz_attr;
+    // Protected by the import lock. The owner fields remember the first
+    // globals/key pair until resolution succeeds or the proxy is cleared.
     PyObject *lz_owner_globals;
     PyObject *lz_owner_key;
     PyObject *lz_resolved;
diff --git a/Lib/test/test_lazy_import/__init__.py b/Lib/test/test_lazy_import/__init__.py
@@ -574,6 +574,166 @@ def custom_import(name, globals=None, locals=None, fromlist=None,
         """)
         assert_python_ok("-c", code)
 
+    @support.requires_subprocess()
+    def test_module_attr_dynamic_equal_name_clears_owner(self):
+        code = textwrap.dedent("""
+            import builtins
+            import sys
+            import types
+
+            real_import = builtins.__import__
+            module = sys.modules[__name__]
+
+            lazy import target_module as target
+            proxy = globals()["target"]
+            stored_name = next(name for name in globals() if name == "target")
+            dynamic_name = bytearray(b"target").decode()
+            assert dynamic_name == stored_name
+            assert dynamic_name is not stored_name
+
+            def custom_import(name, globals=None, locals=None, fromlist=None,
+                              level=0):
+                if name == "target_module":
+                    resolved = types.ModuleType(name)
+                    resolved.VALUE = "resolved"
+                    return resolved
+                return real_import(name, globals, locals, fromlist, level)
+
+            builtins.__import__ = custom_import
+            try:
+                resolved = getattr(module, dynamic_name)
+                assert resolved.VALUE == "resolved"
+                assert module.__dict__["target"] is resolved
+
+                module.__dict__["target"] = proxy
+                again = proxy.resolve()
+                assert again is resolved
+                assert module.__dict__["target"] is proxy
+            finally:
+                builtins.__import__ = real_import
+        """)
+        assert_python_ok("-c", code)
+
+    @support.requires_subprocess()
+    def test_rebound_back_to_same_proxy_after_stale_resolve_does_not_replace(self):
+        code = textwrap.dedent("""
+            import builtins
+            import types
+
+            real_import = builtins.__import__
+            calls = []
+
+            lazy import target_module as target
+            proxy = globals()["target"]
+
+            def custom_import(name, globals=None, locals=None, fromlist=None,
+                              level=0):
+                if name == "target_module":
+                    calls.append(name)
+                    resolved = types.ModuleType(name)
+                    resolved.VALUE = "resolved"
+                    return resolved
+                return real_import(name, globals, locals, fromlist, level)
+
+            builtins.__import__ = custom_import
+            try:
+                sentinel = object()
+                globals()["target"] = sentinel
+                resolved = proxy.resolve()
+                assert globals()["target"] is sentinel
+
+                globals()["target"] = proxy
+                again = proxy.resolve()
+                assert again is resolved
+                assert globals()["target"] is proxy
+                assert len(calls) == 1, calls
+            finally:
+                builtins.__import__ = real_import
+        """)
+        assert_python_ok("-c", code)
+
+    @support.requires_subprocess()
+    def test_resolve_replaces_unaliased_import_global(self):
+        code = textwrap.dedent("""
+            import builtins
+            import types
+
+            real_import = builtins.__import__
+
+            lazy import target_module
+            proxy = globals()["target_module"]
+
+            def custom_import(name, globals=None, locals=None, fromlist=None,
+                              level=0):
+                if name == "target_module":
+                    resolved = types.ModuleType(name)
+                    resolved.VALUE = "resolved"
+                    return resolved
+                return real_import(name, globals, locals, fromlist, level)
+
+            builtins.__import__ = custom_import
+            try:
+                resolved = proxy.resolve()
+                assert resolved.VALUE == "resolved"
+                assert globals()["target_module"] is resolved
+            finally:
+                builtins.__import__ = real_import
+        """)
+        assert_python_ok("-c", code)
+
+    @support.requires_subprocess()
+    def test_resolve_replaces_unaliased_dotted_import_binding(self):
+        code = textwrap.dedent("""
+            import types
+
+            lazy import test.test_lazy_import.data.pkg.bar
+            proxy = globals()["test"]
+
+            resolved = proxy.resolve()
+            assert isinstance(resolved, types.ModuleType)
+            assert resolved.__name__ == "test"
+            assert globals()["test"] is resolved
+            assert "test.test_lazy_import.data.pkg" not in globals()
+        """)
+        assert_python_ok("-c", code)
+
+    @support.requires_subprocess()
+    def test_copying_lazy_proxy_to_second_global_keeps_first_owner(self):
+        code = textwrap.dedent("""
+            import builtins
+            import types
+
+            real_import = builtins.__import__
+            calls = []
+
+            lazy import target_module as primary
+            secondary = globals()["primary"]
+            proxy = globals()["primary"]
+
+            def custom_import(name, globals=None, locals=None, fromlist=None,
+                              level=0):
+                if name == "target_module":
+                    calls.append(name)
+                    resolved = types.ModuleType(name)
+                    resolved.VALUE = "resolved"
+                    return resolved
+                return real_import(name, globals, locals, fromlist, level)
+
+            builtins.__import__ = custom_import
+            try:
+                resolved = proxy.resolve()
+                assert globals()["primary"] is resolved
+                assert globals()["secondary"] is proxy
+
+                again = globals()["secondary"].resolve()
+                assert again is resolved
+                assert globals()["secondary"] is proxy
+                assert len(calls) == 1, calls
+            finally:
+                builtins.__import__ = real_import
+        """)
+        assert_python_ok("-c", code)
+
 
 class SyntaxRestrictionTests(LazyImportTestCase):
     """Tests for syntax restrictions on lazy imports."""
diff --git a/Objects/lazyimportobject.c b/Objects/lazyimportobject.c
@@ -9,6 +9,7 @@
 #include "pycore_interpframe.h"
 #include "pycore_lazyimportobject.h"
 #include "pycore_modsupport.h"
+#include "pycore_unicodeobject.h"
 
 #define PyLazyImportObject_CAST(op) ((PyLazyImportObject *)(op))
 
@@ -141,6 +142,8 @@ _PyLazyImport_BindGlobal(PyThreadState *tstate, PyObject *op,
     PyLazyImportObject *m = PyLazyImportObject_CAST(op);
     PyInterpreterState *interp = tstate->interp;
     _PyImport_AcquireLock(interp);
+    // First owner wins: copies of the proxy do not change the original global
+    // binding that resolve() may replace.
     if (m->lz_resolved == NULL &&
         m->lz_owner_globals == NULL &&
         m->lz_owner_key == NULL)
@@ -157,7 +160,10 @@ clear_owner_if_matches(PyThreadState *tstate, PyLazyImportObject *m,
 {
     PyInterpreterState *interp = tstate->interp;
     _PyImport_AcquireLock(interp);
-    if (m->lz_owner_globals == globals && m->lz_owner_key == name) {
+    if (m->lz_owner_globals == globals &&
+        m->lz_owner_key != NULL &&
+        _PyUnicode_Equal(m->lz_owner_key, name))
+    {
         Py_CLEAR(m->lz_owner_globals);
         Py_CLEAR(m->lz_owner_key);
     }
@@ -178,20 +184,27 @@ _PyLazyImport_CommitIfCurrent(PyThreadState *tstate, PyObject *op,
 
     PyObject *current = NULL;
     int err = 0;
-
+    int clear_owner = 0;
     Py_BEGIN_CRITICAL_SECTION(globals);
     int found = _PyDict_GetItemRef_Unicode_LockHeld(
         (PyDictObject *)globals, name, &current);
     if (found < 0) {
         err = -1;
     }
-    else if (found > 0 && current == op) {
-        err = _PyDict_SetItem_LockHeld((PyDictObject *)globals, name, value);
+    else {
+        // Stop tracking the owner after a successful reification, so a stale
+        // proxy cannot replace a future rebinding.
+        clear_owner = 1;
+        if (found > 0 && current == op) {
+            err = _PyDict_SetItem_LockHeld(
+                (PyDictObject *)globals, name, value);
+            clear_owner = (err == 0);
+        }
     }
-    Py_XDECREF(current);
     Py_END_CRITICAL_SECTION();
+    Py_XDECREF(current);
 
-    if (err == 0) {
+    if (clear_owner) {
         clear_owner_if_matches(tstate, PyLazyImportObject_CAST(op),
                                globals, name);
     }
@@ -211,6 +224,8 @@ _PyLazyImport_CommitStoredIfCurrent(PyThreadState *tstate, PyObject *op,
     PyObject *name = NULL;
 
     PyInterpreterState *interp = tstate->interp;
+    // Copy the owner under the import lock, then release it before entering a
+    // dict critical section in _PyLazyImport_CommitIfCurrent().
     _PyImport_AcquireLock(interp);
     if (m->lz_owner_globals != NULL && m->lz_owner_key != NULL) {
         globals = Py_NewRef(m->lz_owner_globals);
@@ -246,7 +261,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 actual object")
     },
     {NULL, NULL}
 };
@@ -258,9 +273,9 @@ PyDoc_STRVAR(lazy_import_doc,
 "\n"
 "Represents a lazy import that will be resolved on first use.\n"
 "\n"
-"Instances of this object accessed from the global scope will be\n"
-"automatically imported based upon their name and then replaced with\n"
-"the imported value.");
+"A successful resolution is cached. Instances of this object accessed\n"
+"from the global scope will be automatically imported based upon their\n"
+"name and then replaced with the imported value.");
 
 PyTypeObject PyLazyImport_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
