From 29a5951b1814ee885cbe9e94556fe7796b1ad65e Mon Sep 17 00:00:00 2001 From: swayaminsync Date: Tue, 12 May 2026 20:47:44 +0530 Subject: [PATCH 1/3] Fix free-threaded race in importlib lazy-submodule fast path --- Lib/importlib/_bootstrap.py | 43 +++++---- .../test_importlib/test_threaded_import.py | 87 +++++++++++++++++++ 2 files changed, 115 insertions(+), 15 deletions(-) diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 06dc45d71d7ca4..59163389bf3dfe 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -918,6 +918,7 @@ def _load_unlocked(spec): del sys.modules[spec.name] except KeyError: pass + spec._initializing = False raise # Move the module to the end of sys.modules. # We don't ensure that the import-related module attributes get @@ -926,8 +927,10 @@ def _load_unlocked(spec): module = sys.modules.pop(spec.name) sys.modules[spec.name] = module _verbose_message('import {!r} # {!r}', spec.name, spec.loader) - finally: + except: + # Any other exception bubbles up; clear the flag too. spec._initializing = False + raise return module @@ -942,7 +945,10 @@ def _load(spec): """ with _ModuleLockManager(spec.name): - return _load_unlocked(spec) + try: + return _load_unlocked(spec) + finally: + spec._initializing = False # Loaders ##################################################################### @@ -1306,20 +1312,24 @@ def _find_and_load_unlocked(name, import_): finally: if parent_spec: parent_spec._uninitialized_submodules.pop() - if parent: - # Set the module as an attribute on its parent. - parent_module = sys.modules[parent] + try: + if parent: + # Set the module as an attribute on its parent. + parent_module = sys.modules[parent] + try: + setattr(parent_module, child, module) + except AttributeError: + msg = (f"Cannot set an attribute on {parent!r} for child " + f"module {child!r}") + _warnings.warn(msg, ImportWarning) + # Set attributes to lazy submodules on the module. try: - setattr(parent_module, child, module) - except AttributeError: - msg = f"Cannot set an attribute on {parent!r} for child module {child!r}" + _imp._set_lazy_attributes(module, name) + except Exception as e: + msg = f"Cannot set lazy attributes on {name!r}: {e!r}" _warnings.warn(msg, ImportWarning) - # Set attributes to lazy submodules on the module. - try: - _imp._set_lazy_attributes(module, name) - except Exception as e: - msg = f"Cannot set lazy attributes on {name!r}: {e!r}" - _warnings.warn(msg, ImportWarning) + finally: + spec._initializing = False return module @@ -1486,7 +1496,10 @@ def _builtin_from_name(name): spec = BuiltinImporter.find_spec(name) if spec is None: raise ImportError('no built-in module named ' + name) - return _load_unlocked(spec) + try: + return _load_unlocked(spec) + finally: + spec._initializing = False def _setup(sys_module, _imp_module): diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index 6875fdca9c8528..d16fbcfc045a63 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -461,6 +461,93 @@ def do_import(name): errors, [], f"Import(s) failed on iteration {i}: {errors}") + def test_lazy_submodule_getattr_no_recursion(self): + # Regression test: a package that uses a lazy `__getattr__` doing + # `import pkg.sub as sub` must not recurse to RecursionError when + # another thread first-touches the same submodule during the + # window between `_load_unlocked` clearing `spec._initializing` + # and `_find_and_load_unlocked` running `setattr(parent, child, + # module)`. The race is widened deterministically with a + # threading.Event so the test does not depend on luck. + import importlib._bootstrap as _b + + os.makedirs(os.path.join(TESTFN, "lazypkg", "sub")) + self.addCleanup(shutil.rmtree, TESTFN) + with open(os.path.join(TESTFN, "lazypkg", "__init__.py"), "w") as f: + f.write( + "def __getattr__(attr):\n" + " if attr == 'sub':\n" + " import lazypkg.sub as sub\n" + " return sub\n" + " raise AttributeError(attr)\n" + ) + with open(os.path.join(TESTFN, "lazypkg", "sub", "__init__.py"), "w") as f: + f.write("X = 42\n") + + sys.path.insert(0, TESTFN) + self.addCleanup(sys.path.remove, TESTFN) + for mod in ("lazypkg", "lazypkg.sub"): + self.addCleanup(forget, mod) + importlib.invalidate_caches() + + gate = threading.Event() + done = threading.Event() + orig_find_and_load_unlocked = _b._find_and_load_unlocked + + def widen(name, import_): + if name != "lazypkg.sub": + return orig_find_and_load_unlocked(name, import_) + parent_module = sys.modules["lazypkg"] + spec = _b._find_spec(name, parent_module.__path__) + parent_module.__spec__._uninitialized_submodules.append("sub") + try: + module = _b._load_unlocked(spec) + finally: + parent_module.__spec__._uninitialized_submodules.pop() + # Pause inside the unsafe window so the observer thread + # can probe the half-initialized state. Without the fix + # in _bootstrap, that probe drives `__getattr__` into + # infinite recursion. + gate.set() + done.wait(timeout=5.0) + setattr(parent_module, "sub", module) + return module + + _b._find_and_load_unlocked = widen + self.addCleanup(setattr, _b, "_find_and_load_unlocked", + orig_find_and_load_unlocked) + + result = {} + + def loader(): + __import__("lazypkg").sub + + def observer(): + gate.wait(timeout=5.0) + try: + result["value"] = __import__("lazypkg").sub + except BaseException as exc: + result["error"] = exc + done.set() + + t_loader = threading.Thread(target=loader) + t_observer = threading.Thread(target=observer) + t_observer.start() + t_loader.start() + t_loader.join(timeout=10) + t_observer.join(timeout=10) + + self.assertFalse(t_loader.is_alive(), "loader thread deadlocked") + self.assertFalse(t_observer.is_alive(), "observer thread deadlocked") + self.assertNotIn( + "error", result, + f"observer thread raised {type(result.get('error')).__name__}: " + f"{result.get('error')!r}", + ) + self.assertIs( + result["value"], sys.modules["lazypkg.sub"], + "observer thread did not get the loaded submodule") + def setUpModule(): thread_info = threading_helper.threading_setup() From 19bc144bf3e4963c4eafbd2cac48245593186796 Mon Sep 17 00:00:00 2001 From: swayaminsync Date: Tue, 12 May 2026 21:06:01 +0530 Subject: [PATCH 2/3] Add Misc/NEWS.d entry for gh-149728 --- .../Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst diff --git a/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst b/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst new file mode 100644 index 00000000000000..6a633b91eba5ea --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst @@ -0,0 +1,7 @@ +Fix a race in :mod:`importlib._bootstrap` where the fast path in +``_find_and_load`` could return a module whose ``parent.child`` attribute had +not yet been set. Under free-threaded CPython this caused packages using the +lazy-submodule ``__getattr__`` pattern (e.g. NumPy) to recurse to +:exc:`RecursionError` when multiple threads first-touched the same lazy +submodule concurrently. ``spec._initializing`` is now kept ``True`` until +after ``_find_and_load_unlocked`` has attached the module to its parent. From 1feb29262440d3227cb37979a6d540e3b6eb528b Mon Sep 17 00:00:00 2001 From: Swayam Date: Tue, 12 May 2026 23:26:19 +0530 Subject: [PATCH 3/3] brief the news Fixes a race condition in importlib's module loading when using lazy submodules, preventing RecursionError in multi-threaded scenarios. --- .../2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst b/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst index 6a633b91eba5ea..9e1db1a19fce0a 100644 --- a/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst +++ b/Misc/NEWS.d/next/Library/2026-05-12-22-00-00.gh-issue-149728.LzyImp.rst @@ -1,7 +1 @@ -Fix a race in :mod:`importlib._bootstrap` where the fast path in -``_find_and_load`` could return a module whose ``parent.child`` attribute had -not yet been set. Under free-threaded CPython this caused packages using the -lazy-submodule ``__getattr__`` pattern (e.g. NumPy) to recurse to -:exc:`RecursionError` when multiple threads first-touched the same lazy -submodule concurrently. ``spec._initializing`` is now kept ``True`` until -after ``_find_and_load_unlocked`` has attached the module to its parent. +fix race in importlib when importing module which defined getattr